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

Demix NavigatorConcurrentHardware #9079

Closed
wants to merge 1 commit into from

Conversation

foolip
Copy link
Collaborator

@foolip foolip commented Feb 12, 2021

Part of #8929

@foolip foolip requested a review from Elchi3 February 12, 2021 23:22
@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Feb 12, 2021
@foolip foolip force-pushed the demix-NavigatorConcurrentHardware branch from 1c5b836 to cd5f7bf Compare February 12, 2021 23:23
@foolip
Copy link
Collaborator Author

foolip commented Feb 12, 2021

This is simply splitting the entry without changes. I checked the data against mdn-bcd-collector results and didn't find any differences for Worker. For WorkerNavigator there was a proposed change, but it doesn't look legit, I filed foolip/mdn-bcd-collector#960.

@foolip
Copy link
Collaborator Author

foolip commented Feb 12, 2021

@Elchi3 changes like this will require a lot of fixes on MDN. I'd like to not volunteer for that, but is it useful for me to send PRs like this one?

api/Navigator.json Outdated Show resolved Hide resolved
@Elchi3
Copy link
Member

Elchi3 commented Feb 14, 2021

@Elchi3 changes like this will require a lot of fixes on MDN. I'd like to not volunteer for that, but is it useful for me to send PRs like this one?

I'm trying to burn through all mixins on BCD and on MDN. Any help is useful :)

@foolip
Copy link
Collaborator Author

foolip commented Feb 14, 2021

Before this is merged I want to note that WorkerNavigator and WorkerLocation are odd cases and it's not obvious how MDN/BCD should represent them. These interfaces predate the use of [Exposed=(Window,Worker)] and are subset copies of Navigator and Location.

The thing is that even though these interfaces are real, web developers just use navigator.language or location.href in both window and worker scripts. I'm skeptical that it makes sense to document them separately, and am unsure if spitting them on BCD is the right thing.

@foolip
Copy link
Collaborator Author

foolip commented Feb 14, 2021

Or rather I should say I could see us changing this later to use the same mechanism for representing global scope exposure as for interfaces and global methods, assuming a single method for that emerges after WindowOrWorkerGlobalScope demixing.

@Elchi3
Copy link
Member

Elchi3 commented Feb 14, 2021

hm, I see. Can we file a separate issue that discusses options for the whole Window/Worker problem with mixins? We can wait to merge this PR until we have had that discussion if you like.

@foolip
Copy link
Collaborator Author

foolip commented Feb 15, 2021

I've filed #9125 for this specific issue, and #9127 for WindowOrWorkerGlobalScope.

@ddbeck ddbeck requested a review from Elchi3 February 18, 2021 13:29
Base automatically changed from master to main March 24, 2021 12:54
@foolip foolip force-pushed the demix-NavigatorConcurrentHardware branch from ee2dcdf to 59d9567 Compare June 7, 2021 07:50
@foolip foolip added not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. and removed rebase needed 🚧 labels Jun 7, 2021
@foolip
Copy link
Collaborator Author

foolip commented Jun 7, 2021

I've rebased this now but I don't like it. It's a similar issue to demixing WindowOrWorkerGlobalScope, where web developers can use the same code in windows and workers, and splitting it into two entries seems unhelpful. Finding a way to represent this as one entry meaning navigator.hardwareConcurrency and using notes or partial_implementation if it doesn't work in workers seems better.

@foolip foolip marked this pull request as draft June 8, 2021 10:12
@Elchi3
Copy link
Member

Elchi3 commented Jun 9, 2021

Thanks Philip! I propose we sort out all other mixins first and then look at everything that touches the window and worker problematic.

@Elchi3
Copy link
Member

Elchi3 commented Jul 8, 2021

This was done in #11413

@foolip foolip closed this Jul 12, 2021
@foolip foolip deleted the demix-NavigatorConcurrentHardware branch July 12, 2021 17:56
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 not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants