Skip to content

Use the babel-plugin-polyfill-* packages in transform-runtime#12845

Merged
nicolo-ribaudo merged 5 commits intobabel:mainfrom
nicolo-ribaudo:runtime-use-polyfills-packages
Feb 22, 2021
Merged

Use the babel-plugin-polyfill-* packages in transform-runtime#12845
nicolo-ribaudo merged 5 commits intobabel:mainfrom
nicolo-ribaudo:runtime-use-polyfills-packages

Conversation

@nicolo-ribaudo
Copy link
Copy Markdown
Member

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

This is the same as #12583, but for @babel/runtime 🙂

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Feb 22, 2021
// This file isn't maintaned anymore.
// Improvements should go in the babel-plugin-polyfill-corejs2 package.

export default {
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.

These two files have just been moved from src to scripts. GitHub shows them as new because I changed the indentation.

}
if (!useRuntimeHelpers) return;

file.set("helperGenerator", name => {
Copy link
Copy Markdown
Member Author

@nicolo-ribaudo nicolo-ribaudo Feb 22, 2021

Choose a reason for hiding this comment

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

This code has only been de-indented. You can review with https://github.com/babel/babel/pull/12845/files?w=1 to disable whitespace changes.

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented Feb 22, 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 ac5ffcb:

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

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented Feb 22, 2021

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

Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

LGTM overall except for comments.

var _regeneratorRuntime = require("@babel/runtime-corejs3/regenerator");

var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map");
var _mapInstanceProperty = require("@babel/runtime-corejs3/core-js/instance/map.js");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should not generate package/path/polyfill.js when absoluteRuntime is true, it should be an absolute path to the polyfill.

var _Symbol = require("@babel/runtime-corejs3/core-js-stable/symbol.js");

var _Promise = require("@babel/runtime-corejs3/core-js-stable/promise");
var _Map = require("@babel/runtime-corejs3/core-js-stable/map.js");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the extension .js should be added since we have specified package.json#exports.

var _regeneratorRuntime = require("<CWD>/packages/babel-runtime-corejs3/regenerator");

var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map");
var _mapInstanceProperty = require("<CWD>/packages/babel-runtime-corejs3/core-js/instance/map.js");
Copy link
Copy Markdown
Member Author

@nicolo-ribaudo nicolo-ribaudo Feb 22, 2021

Choose a reason for hiding this comment

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

Here the extension is ok correct, because when using absolute or relative paths npm ignores package.json#exports (I added it for consistency with the corejs2 transform).

This is currently broken for helpers, but the fix falls in unrelated from this PR.

@nicolo-ribaudo nicolo-ribaudo merged commit 484667b into babel:main Feb 22, 2021
@nicolo-ribaudo nicolo-ribaudo deleted the runtime-use-polyfills-packages branch February 22, 2021 21:30
@merceyz
Copy link
Copy Markdown
Contributor

merceyz commented Feb 23, 2021

This seems to fix #12106 and #10759

@nicolo-ribaudo
Copy link
Copy Markdown
Member Author

At least not #12106, that needs to be fixed in regenerator itself.

This was referenced Mar 13, 2021
@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 May 25, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 25, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants