Skip to content

Inject core-js@3 imports in Program:exit instead of on post()#10146

Merged
nicolo-ribaudo merged 4 commits intobabel:masterfrom
nicolo-ribaudo:issue-10142
Oct 15, 2019
Merged

Inject core-js@3 imports in Program:exit instead of on post()#10146
nicolo-ribaudo merged 4 commits intobabel:masterfrom
nicolo-ribaudo:issue-10142

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo commented Jun 29, 2019

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

Nodes injected during post() can't be visited again, so it should be avoided.

When reviewing this PR, disable whitespaces in the diff.

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: preset-env labels Jun 29, 2019
@babel-bot
Copy link
Copy Markdown
Collaborator

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

@Jessidhia
Copy link
Copy Markdown
Member

I wonder how this works with the combination of builtins-usage and the commonjs transform.

The commonjs transform uses Object.defineProperty and it's possible preset-env detects it as something to be polyfilled.

@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

In that case, after modules have been transformed, useBuiltIns will inject a plain require call to the polyfill.
I believe we have a test for this, let me check.

@@ -1,2 +1,2 @@
import "core-js/modules/es.object.from-entries";
import "core-js/modules/esnext.string.replace-all";
import 'core-js/modules/es.object.from-entries';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These quote changes seem unintended/unnecessary?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's because Babel, when possible, reuses the original quotes.
The old implementation removed this import statements and replaced with a new copy of them, while this implementation avoids replacing an import which is already ok.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

This allows them to be requeued and transformed by other plugins.
…s [skip ci]

Co-Authored-By: Brian Ng <bng412@gmail.com>
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 pkg: preset-env PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

How to transform output of presets?

5 participants