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

Build: Change the source for wp-polyfill #1361

Closed
wants to merge 2 commits into from
Closed

Build: Change the source for wp-polyfill #1361

wants to merge 2 commits into from

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jun 9, 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 June 9, 2021 19:29
@gziolo gziolo self-assigned this Jun 9, 2021
@gziolo gziolo added the dependencies Pull requests that update a dependency file label Jun 9, 2021
@gziolo
Copy link
Member Author

gziolo commented Jun 9, 2021

I tested on macOS Big Sur with the latest:

  • Firefox
  • Safari
  • Chrome

Everything works as expected.

@gziolo gziolo requested a review from youknowriad June 9, 2021 19:32
@youknowriad
Copy link
Contributor

Looks good, what's the different in terms of Kb?

@gziolo
Copy link
Member Author

gziolo commented Jun 11, 2021

When I move to dependencies:

"@wordpress/babel-preset-default": "6.2.0",

I get the following error:

Screen Shot 2021-06-11 at 11 36 42

I'm not sure why it's happening :( Can we keep it as devDependencie?

@youknowriad
Copy link
Contributor

I'm not sure why it's happening :( Can we keep it as devDependencie?

the error is weird but yeah we can keep there if it solves it.

Copy link
Contributor

@desrosj desrosj left a comment

Choose a reason for hiding this comment

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

This is looking good to me. Thanks for working on this @gziolo!

pierlon added a commit to ampproject/amp-wp that referenced this pull request Jun 14, 2021
@desrosj
Copy link
Contributor

desrosj commented Jun 14, 2021

Just wanted to record the difference in file size here for historical reference:

Before

Unminified file: 242KB
Minified file: 102KB

After

Unminified file: 53KB
Minified file: 20KB

@desrosj
Copy link
Contributor

desrosj commented Jun 14, 2021

Merged into Core in https://core.trac.wordpress.org/changeset/51146.

@desrosj desrosj closed this Jun 14, 2021
@gziolo gziolo deleted the update/babel-polyfill branch June 15, 2021 06:28
pierlon added a commit to ampproject/amp-wp that referenced this pull request Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
3 participants