Skip to content

Build: Remove @babel/polyfill in favor of core-js/stable#1191

Closed
gziolo wants to merge 1 commit intoWordPress:masterfrom
gziolo:update/babel-polyfill
Closed

Build: Remove @babel/polyfill in favor of core-js/stable#1191
gziolo wants to merge 1 commit intoWordPress:masterfrom
gziolo:update/babel-polyfill

Conversation

@gziolo
Copy link
Copy Markdown
Member

@gziolo gziolo commented Apr 13, 2021

Trac ticket: https://core.trac.wordpress.org/ticket/52941


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@gziolo gziolo requested a review from desrosj April 13, 2021 15:55
@gziolo gziolo self-assigned this Apr 13, 2021
const vendors = {
'lodash.js': 'lodash/lodash.js',
'wp-polyfill.js': '@babel/polyfill/dist/polyfill.js',
'wp-polyfill-core-js.js': 'core-js/stable/index.js',
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.

@desrosj, I need to do some research because it doesn't look like core-js has a distribution version but rather a long list of require calls ...

}

$scripts->add( 'wp-polyfill', null, array( 'wp-polyfill' ) );
$scripts->add( 'wp-polyfill', null, array( 'wp-polyfill-core-js', 'wp-polyfill-regenerator' ) );
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 looks good. We need to be careful to not change the script handle. I don't think the other changes above will have any affect on that. But we should just double check.

@sgomes
Copy link
Copy Markdown

sgomes commented Apr 27, 2021

Hey @gziolo! Thanks for this PR! 👍

I just wanted to bring up a note I left on the Trac ticket around the aggressiveness of core-js polyfilling. It tends to be extremely conservative regarding full feature correctness, even for rarely used things. It may be worth customising the aggressiveness a bit to avoid e.g. shipping a Promise polyfill unnecessarily.

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Apr 28, 2021

@sgomes, thanks for the pointer. I arrived at the same documentation before reading your comment when looking at how to reword notes in WordPress packages after dropping support for IE11.

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Apr 28, 2021

I'm exploring a completely revised approach in WordPress/gutenberg#31279.

@desrosj
Copy link
Copy Markdown
Member

desrosj commented May 24, 2021

The revised approach was merged into the Gutenberg repository and will be merged to Core.

@desrosj desrosj closed this May 24, 2021
@gziolo gziolo deleted the update/babel-polyfill branch May 24, 2021 13:32
@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented May 24, 2021

Yes, good catch. I will try to publish the change to npm so we can try again using the polyfill from @wordpress/babel-preset-default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants