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

add export * as ns support data #6394

Merged
merged 4 commits into from Jul 23, 2020
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jul 13, 2020

A checklist to help your pull request get merged faster:

  • Summarize your changes
  • Data: link to resources that verify support information (such as browser's docs, changelogs, source control, bug trackers, and tests)
  • Review the results of the linter and fix problems reported (If you need help, please ask in a comment!)

Context
Babel 7.11 will detect whether export * as ns should be transformed according to compat-data here: babel/babel#11849

Sources:
V8: https://v8.dev/blog/v8-release-72#module-namespace-exports
SM: https://bugzilla.mozilla.org/show_bug.cgi?id=1496852, https://groups.google.com/g/mozilla.dev.platform/c/NSzWY92Ut0o?pli=1
JSC: https://bugs.webkit.org/show_bug.cgi?id=214379

Environments Mapping: https://github.com/mdn/browser-compat-data/tree/master/browsers

@ghost ghost added the data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript label Jul 13, 2020
@JLHwung JLHwung force-pushed the add-export-namespace branch 2 times, most recently from 930b317 to dbdae59 Compare July 14, 2020 21:22
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR, @JLHwung! The data checks out nicely—good research.

I do have a structural change to suggest, though. I think the feature should be a direct subfeature of export (at the same level as default, just before the current feature). That will allow simplifying some the data as well—see the line comment for details.

Thank you again!

javascript/statements.json Outdated Show resolved Hide resolved
javascript/statements.json Outdated Show resolved Hide resolved
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JLHwung
Copy link
Contributor Author

JLHwung commented Jul 16, 2020

I think the feature should be a direct subfeature of export (at the same level as default, just before the current feature)

namespace is indeed a subfeature of export.

we won't need the flags data here. It can be just be "version_added": "12.0.0"

Did you mean version_added: "13.2.0"? Otherwise as a compat-table consumer, the data will be

  • export available (without flags) from 13.2.0
  • export namespace available (no flags informations) from 12.0.0.

Also export.default has flags data, too. I can remove them if decision is made, but would want to clarify a bit.

@ddbeck
Copy link
Collaborator

ddbeck commented Jul 20, 2020

namespace is indeed a subfeature of export.

Ah, my apologies. I misread the folding in my editor.

Did you mean version_added: "13.2.0"? Otherwise as a compat-table consumer, the data will be

  • export available (without flags) from 13.2.0
  • export namespace available (no flags informations) from 12.0.0.

No, I meant from 12.0.0. When it comes to flags, subfeatures present a slightly different story. With APIs, as opposed to language features, it's a little more obvious: if some SomeAPI is behind a flag, then SomeAPI.someMethod() is also behind the flag, but there's typically no additional flag needed for someMethod() to be enabled. Duplicating the flag data on someMethod() is a bit misleading because you can't switch on someMethod() alone. It's only gated through the parent feature.

I understood this to be a similar situation: you can't turn on the namespace feature independent of the parent modules feature.

This is an area where we could stand to improve the documentation for the schema and contributions.

Also export.default has flags data, too. I can remove them if decision is made, but would want to clarify a bit.

Yeah, this is a bit inconsistent. I'd welcome a PR fixing that too. It doesn't have to be part of this change, though.

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you! 🎉

@ddbeck ddbeck merged commit 02d8a70 into mdn:master Jul 23, 2020
@JLHwung JLHwung deleted the add-export-namespace branch July 23, 2020 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants