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

Tag everything with worker support? #7849

Open
hamishwillee opened this issue Dec 15, 2020 · 17 comments
Open

Tag everything with worker support? #7849

hamishwillee opened this issue Dec 15, 2020 · 17 comments
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API idle 🐌 Issues and pull requests with no recent activity

Comments

@hamishwillee
Copy link
Collaborator

Worker support of APIs is quite a challenge to track and maintain docs for - it can vary across an API and be spotty across browsers. It also isn't necessarily obvious to a user how they go about using some APIs.

I think it might make sense to "standardise" this in BCD and roll it out more or less for every single Web API.

  • There are four types of workers - dedicated, shared, service, chrome. I don't know if we'd need to indicate these separately or not.
  • It is useful to note the API used to access the feature in both main thread (often via window or window.navigator) and the web service (usually some context)

There are a few good examples where this has been done in an adhoc way:

FYI @chrisdavidmills @Elchi3 @ddbeck Would appreciate your thinking on this. To me this is a similar problem to documenting secure contexts.

@queengooborg queengooborg added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Dec 15, 2020
@ddbeck
Copy link
Collaborator

ddbeck commented Dec 15, 2020

You're right that we ought to be more consistent about this. A couple thoughts on how we might do this:

First, I suppose we should have a worker support entry for things which have been specified as having worker support (stuff with [Exposed=(Window,Worker)] and the like in the IDL), or things which have worker support and don't have a specification (or for features for which either of these statements was historically true). We probably don't need a worker_support entry for every feature. All that said, I don't think we've done a great job of reviewing for this specifically. Perhaps this might be of interest to @foolip and @vinyldarkscratch's testing efforts.

Second, when documenting worker support, we should use the worker_support entry. We have a data guideline for this, which makes both the service_workers_support entry in FileReaderSync incorrect (it should be deleted in favor of worker_support), as well as the notes on CacheStorage (it should have a separate entry for this). If you run into more unusual data like this, I'd welcome issues or PRs to address them.

@foolip
Copy link
Collaborator

foolip commented Dec 15, 2020

https://github.com/foolip/mdn-bcd-collector does test for support in workers, but the scripts don't yet update the worker_support entries. I think that's pretty doable though.

One problem we'll have here is with global methods like fetch() and attributes like isSecureContext. These are usually in Window.json, WorkerGlobalScope.json or WindowOrWorkerGlobalScope.json. And there are also 6 other *GlobalScope.json files in BCD. These entries have an implied support in certain contexts, but the support might not actually be there. There's just a single worker_support entry, for https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/indexedDB.

We'd also need to figure out a way to represent that an interface is exposed in workers, but that certain of its members are not. This is rare, but happens, for example with responseXML in https://xhr.spec.whatwg.org/#interface-xmlhttprequest. The complication is that we can't just let worker_support on a parent feature imply support on all child features.

I suspect this issue may also interest @saschanaz.

@saschanaz
Copy link
Contributor

Yes! The TypeScript types generator currently only depends on Web IDL [Exposed] attribute regardless of the real situation, but a general worker_support field will help detecting real worker support.

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Dec 18, 2020

Thanks all. So I guess first thing to do is fix the known broken ones. Started that in #8014. Already a bit stuck with the right way to show that some things are supported in different types of workers in different versions. I simply don't understand BCD well enough to know the right way to do do this yet.

The complication is that we can't just let worker_support on a parent feature imply support on all child features.

That means that we'll have to tag everything and let people know they have to check every feature to be sure if it is implemented - right?

With respect to scopes we perhaps don't need to go to specific class since any global worker context is access by self. I believe navigator exposed by global scope as either self.navigator or just navigator. It is useful to say which one of those you use to access the thingy.

Thoughts?

@foolip
Copy link
Collaborator

foolip commented Dec 19, 2020

That means that we'll have to tag everything and let people know they have to check every feature to be sure if it is implemented - right?

I think there are less than 20 cases of this on the web platform, so while it needs to be handled, we should probably do it in a way that doesn't make all features look complicated to consumers. One way would be to say that worker_support does inherit, but have a way of setting specific subfeatures as not supported in workers. I don't think there's any precedence for inheritance or overrides in BCD though, in all cases I know of (flags, prefixes) there's no consistency or agreement :)

With respect to scopes we perhaps don't need to go to specific class since any global worker context is access by self. I believe navigator exposed by global scope as either self.navigator or just navigator. It is useful to say which one of those you use to access the thingy.

Things are defined on a global scope (Window or *GlobalScope interfaces) don't really need worker_support, no. For the most part one can figure out where they are supported from the name.

I was thinking a few steps into the future with changes I'd like to see:

@ddbeck
Copy link
Collaborator

ddbeck commented Jan 6, 2021

OK, now I have to cop to not knowing very much about workers. Can someone point me toward something that explains the different workers? This has me wondering what our current worker_support features actually refer to.

One way would be to say that worker_support does inherit, but have a way of setting specific subfeatures as not supported in workers.

Maybe we could bless certain features as inherited, then actually cascade them down the tree as part of a compilation step unless a feature already has its own worker-support feature (that is, more specific data would overrule inherited data). Then consumers wouldn't have to learn resolve an inheritance on their own.

@hamishwillee
Copy link
Collaborator Author

@ddbeck
The entry point for find out about the four worker types is https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API#worker_types. For this purpose we probably only need to think about two cases:

  • Dedicated and shared workers (if one is supported, probably both will be)
  • Service workers

Chrome workers are FF only.

Maybe we could bless certain features as inherited, then actually cascade them down the tree as part of a compilation step unless a feature already has its own worker-support feature (that is, more specific data would overrule inherited data). Then consumers wouldn't have to learn resolve an inheritance on their own.

I don't know, but my feeling is "probably".

@hamishwillee
Copy link
Collaborator Author

FYI Added #8783 to sort out CacheStorage

@hamishwillee
Copy link
Collaborator Author

FYI Added #8784 for PushManager.

@github-actions github-actions bot added the idle 🐌 Issues and pull requests with no recent activity label May 25, 2022
@foolip
Copy link
Collaborator

foolip commented Aug 5, 2022

After #11518 we have a bit of a mixed situation in the api/_globals/ folder. Most entries have a "worker_support" entry, except scheduler.json. But for the most part the subfeatures just have the same data, and any differences are just mistakes. structuredClone is the first such case I spotted, where the Chrome and Firefox data match the parent feature, and the Safari data is incorrectly set to false and should match the parent.

I prepared a PR (#17260) to remove subfeatures that I think don't add any information, but then remembered this issue.

So what should we do, always add the subfeature, or only when it adds information?

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Aug 8, 2022

@foolip I think you are saying if that any subfeature has matching information to the parent then it should not be documented. That would imply that if a function of an interface was introduced at the same time, you wouldn't document it. Which I would find annoying because then I'd have no link for the spec, and I think I'd probably assume it was an omission. That might indeed be "what we do" :-(

To me the subfeature does add information - once you remove this there is nothing on the page to tell a user that (say) createImageBitmap() supports workers, and the versions in which it supports works.
Or do I misunderstand what you are saying here?

Assuming I do understand the point, I see this as a little like tagging for secure process. IMO that would be best done following the IDL. Mostly because it makes absolutely clear how things work and requires no thinking from anyone.

@foolip
Copy link
Collaborator

foolip commented Aug 10, 2022

@hamishwillee that isn't quite what I mean, no. Worker support should definitely be documented, on MDN.

I think about this similarly to returns_promise or something_parameter subfeatures. The spec requires something, and a lot of the time it's been like that in spec and implementations since the start. Adding those cases to BCD doesn't add any information, but would add distraction, and we don't do it.

Worker support is bit different because it's been so common that worker support came later in the past. It hasn't been as common lately though, and I'd be surprised if this trend reversed.

@hamishwillee
Copy link
Collaborator Author

@foolip Ah, thanks. So "yes" to all you said. Your PR is correct.

Workers are currently treated the same way as everything else in BCD. As you say, they should only get a worker subfeature if support in workers appeared at a different release than the feature (in one or more browser).

That said, the root problem still exists - we need to properly document when things are supported by workers, and that needs to match the IDL. But the summary of this thread should probably should be "the place worker support info comes form is not BCD".

Should we close this issue and reopen on the mdn or yari repo? My thinking is that it would be ideal if yari could get the information direct from IDL on a regular basis and automatically add a worker support note based on that data. If not, if we could have a page metadata for it and tooling to match up IDL and docs on a regular basis.

@foolip
Copy link
Collaborator

foolip commented Aug 12, 2022

@hamishwillee right, that information should come from https://www.npmjs.com/package/@webref/idl, I'd say.

But I think we still need to update https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines.md#web-workers-worker_support to say when the subfeature should be added and not.

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Aug 15, 2022

@foolip I did a first shot at updating the data guidelines in #17352

I did it generally rather than specific to worker support, because that was missing. The example I used was a bit irritating because it does not comply with the rules we have discussed here. So I think we need confirmation of that PR before we close this one.

EDIT. Actually the guidelines are not clear.

  • @queengooborg has been using the term subfeature to mean "method or property of the interface", and she expects those to all be documented. She is not so clear on what this means for "behaviours" like worker support.
  • I have been using the term subfeature to mean "any json node: below the top level feature node. So methods, properties, behaviours, anything. So we have been talking at cross purposes
  • She is supposed to be updating the docs today with a proposal that covers this.

@queengooborg
Copy link
Collaborator

Actually, I have been using the term "subfeature" to refer to any JSON node below a top-level feature as well. 🙃 I also use the term "behavioral subfeature" for behaviors, options, etc. -- they're not good terms and I'd love to find better ones!

(P.S. @hamishwillee, you referred to me as a "he", not a "she" -- I changed your above comment to fix the pronouns!)

@hamishwillee
Copy link
Collaborator Author

(P.S. @hamishwillee, you referred to me as a "he", not a "she" -- I changed your above comment to fix the pronouns!)

Thanks so much. Sorry about that. Don't know what was going through my head way back then.

I understand how this works now at least, which is nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API idle 🐌 Issues and pull requests with no recent activity
Projects
None yet
Development

No branches or pull requests

7 participants
@ddbeck @foolip @saschanaz @queengooborg @hamishwillee and others