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 allowTopLevelThis option to transform-modules-systemjs #10780

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Nov 29, 2019

Q                       A
Fixed Issues? Fixes #10775
Major: Breaking Change? See discussion below.
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? Yes, @babel/plugin-transform-modules-systemjs now depends on @babel/helper-module-transforms.
License MIT

This PR adds allowTopLevelThis option to transform-modules-systemjs. To be consistent with other module transforms plugin, the default value of allowTopLevelThis is false, which means the top level this will be replaced by void 0 by default.

The spec dictates that in the module executing context, this is always undefined. So the default value does make it align to the specs. Though I am not sure how many people rely on the previous incompliant behaviour.

  • add document PR if we agree on the interface

@JLHwung JLHwung added PR: New Feature 🚀 A type of pull request used for our changelog categories area: modules labels Nov 29, 2019
@existentialism existentialism added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Dec 21, 2019
@nicolo-ribaudo
Copy link
Member

@JLHwung Docs 🙏

@nicolo-ribaudo nicolo-ribaudo merged commit 44f9d85 into babel:master Jan 10, 2020
@nicolo-ribaudo nicolo-ribaudo deleted the allow-top-level-this-transform-modules-systemjs branch January 10, 2020 02:16
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Apr 10, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: modules outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Needs Docs PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[transform-modules-systemjs] top level this not mapped to undefined
3 participants