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

Align regenerator-transform import with native ESM #13086

Merged
merged 2 commits into from Apr 7, 2021

Conversation

nicolo-ribaudo
Copy link
Member

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

I missed this in #12795 because it was using export ... from rather than import.

regenerator-transform is a compiled CJS module, so the default export in Node.js is module.exports and we'll need to get .default from module.exports (thus .default from the default export).

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Apr 1, 2021
@nicolo-ribaudo nicolo-ribaudo added this to In progress in Move to native ES modules via automation Apr 1, 2021
@babel-bot
Copy link
Collaborator

babel-bot commented Apr 1, 2021

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/45076/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 1, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ebf7a43:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

src => src.startsWith("babel-plugin-polyfill-"),
];
const depsUsing__esModuleAndDefaultExport = src =>
src.startsWith("babel-plugin-polyfill-") || src === "regenerator-transform";
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, is there anything else other than regenerator-transform that would need to be added? Would we need to update this if we added another similar dep?

Oh so via #13017, it's only esm/cjs packages that have been transformed with babel?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the only problem is dependencies that have been transformed with Babel.

I managed to create a working build just with this change so I think it's the only one (also I tried looking for other export ... from of dependencies and this is the only one), but if we'll ever find a new one we can always add it to this list.

Copy link
Member

Choose a reason for hiding this comment

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

Ok was just worried it would break without us knowing and whether one could verify!

@nicolo-ribaudo
Copy link
Member Author

Merging since this is a really small internal change, that I forgot to do in another PR.

The new tests doesn't test changed behavior, but it's just to be 100% sure I didn't accidentally break something.

@nicolo-ribaudo nicolo-ribaudo merged commit 6d89daf into babel:main Apr 7, 2021
Move to native ES modules automation moved this from In progress to Done Apr 7, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the regeneator__esModule branch April 7, 2021 15:25
@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 Jul 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Development

Successfully merging this pull request may close these issues.

None yet

3 participants