Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature detection of module workers is hard #5325

Open
surma opened this issue Mar 2, 2020 · 9 comments
Open

Feature detection of module workers is hard #5325

surma opened this issue Mar 2, 2020 · 9 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: workers

Comments

@surma
Copy link
Contributor

surma commented Mar 2, 2020

Currently, the advised way to feature-detect support for new Worker({type: "module"}) is to create an options object and see if the .type property gets accessed by the constructor with a getter:

let supportsModuleWorker = false;
const options = {
  get type() {
    supportsModuleWorker = true
  }
};
new Worker("data:,", options).terminate();

That seems fairly convoluted. I propose adding type property to module instances:

const w = new Worker("data:,", {type: "module"})
const supportsModuleWorker = "type" in w;
w.terminate();
@developit
Copy link

developit commented Mar 2, 2020

Ideally the property would be a getter defined on the Worker interface itself, which would make testing for it even cheaper:

const supportsModuleWorker = 'type' in Worker.prototype;

This might also be useful for other things aside from checking for support - there might be cases where a Worker is used to load scripts dynamically, and the caller needs to have knowledge of this in order to pass the correct value (a module or classic script URL, perhaps).

@domenic
Copy link
Member

domenic commented Mar 2, 2020

The feature detection snippet you post doesn't seem particularly "hard" to me, and matches how feature detection works for every other option-accepting API in the platform. I think this is not worth the extra work.

@wanderview
Copy link
Member

One complaint with feature detection in general is that its costly. Being able to feature detect module support without having to pay the cost to spin up a worker seems beneficial.

@surma
Copy link
Contributor Author

surma commented Mar 2, 2020

matches how feature detection works for every other option-accepting API in the platform

Personally, I wasn‘t aware that this was a pattern at all. I find it quite unpleasant. But that might just be me then :D

I would want to +1 @wanderview’s comment (and @developit’s suggestion) that if we can avoid spinning up a worker to detect support, that’d be nice.

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: workers labels Mar 2, 2020
@annevk
Copy link
Member

annevk commented Mar 3, 2020

If we add a new value to type, how would you check support without (potentially) spinning up a worker? The suggestions without terminate() only work for this one-off transition. (And even if we did add it, those wanting to support earlier browsers with module worker support would end up with an even more convoluted check.)

(It's unfortunate that t sorts after n, otherwise you could throw from the name getter to avoid instantiation.)

@NotWoods
Copy link

I'll throw out that Chromium WebView on Android reads type despite not supporting module workers, making feature detection even more complicated.
Uncaught (in promise) TypeError: Failed to construct 'Worker': Module scripts are not supported on DedicatedWorker yet. You can try the feature with '--enable-experimental-web-platform-features' flag (see https://crbug.com/680046)

let supportsModuleWorker = false
const options = {
  get type() {
    supportsModuleWorker = true
    return 'module'
  }
}
try {
  new Worker('data:', options)
} catch (err) {
  supportsModuleWorker = false
  // Hope the worker didn't throw for some other reason, or filter out the error
}

@Kaiido
Copy link
Member

Kaiido commented May 24, 2021

Being able to feature detect module support without having to pay the cost to spin up a worker seems beneficial.

Using "blob://" rather than "data:" will avoid spinning the Worker and still call the options' getter. It will throw in Chrome and fire an Error event on the Worker object in FF, but that's what we actually want, the worker scope is not instantiated at all. Related stackoverflow answer

Regarding UAs that don't support this feature but still access the getters, that's indeed a big issue, and this case is not isolated... I was told it should be considered a browser bug but maybe it would be good to write some enforcing rules on WebIDL so this doesn't happen? (Not sure what form this could take...)

But anyway a final problem with "dictionnary traps" is that we can only test if the properties are accessed, not if the value is supported. If in a few years a new WorkerType is added, we would have no way to distinguish its support from "module"'s.

@annevk
Copy link
Member

annevk commented May 25, 2021

@NotWoods did you file a bug on that?

@NotWoods
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: workers
Development

No branches or pull requests

7 participants