fix: use target from webExtConfig if browser option is undefined
#232
Conversation
| : "chromium"; | ||
| pluginOptions.browser === "chrome" | ||
| ? "chromium" | ||
| : pluginOptions.browser === null || |
There was a problem hiding this comment.
I wonder if we should deprecate the browser: null setting. It's a bit cryptic 😅
There was a problem hiding this comment.
Huh, yeah, wonder what I was thinking lol. Let's leave it for now.
| * @default "chrome" | ||
| */ | ||
| browser?: string; | ||
| browser?: 'chrome' | 'firefox' | null; |
There was a problem hiding this comment.
This should remain a string. You can target more than just chrome or firefox. Safari, opera, etc.
There was a problem hiding this comment.
Oops! That's strictly for the manifest templating? I don't see any handling for Safari and others in createWebExtRunner, hence the assumption. 😄
| * @default "chrome" | ||
| */ | ||
| browser?: string; | ||
| browser?: 'chrome' | 'firefox' | (string & {}) | null; |
There was a problem hiding this comment.
this is a TypeScript trick that provides auto-completion for some values while still allowing any string
There was a problem hiding this comment.
Interesting, didn't know this worked... I've only used branded types with a property inside the {}. I've always had to use a function overload to get this behavior before, very good to know!
2 participants
Based on #231 so merge that first.
If the
browserplugin option is undefined, check thewebExtConfig.targetoption before defaulting to chromium.