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.

Sign up for GitHub

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

Jump to bottom

feat(gatsby): add polyfill chunk to gatsby #25159

Merged
merged 3 commits into from Jul 9, 2020
Merged

feat(gatsby): add polyfill chunk to gatsby #25159

merged 3 commits into from Jul 9, 2020

Conversation

wardpeet
Copy link
Contributor

@wardpeet wardpeet commented Jun 20, 2020 •
edited

Description

Creates a separate polyfill chunk that contains all legacy browser modules. We reduce around 40kb of javascript for Modern browsers.

You can test it by running (it's using #25162 as code)

yarn add --registry=https://registry.wardpeet.dev gatsby@polyfills

Gatsby-legacy-polyfills

A package that contains all polyfills necessary for legacy browsers. It get compiled by microbundle so everything is compressed and everything is inside the bundle (no externals). This gives us the opportunity to improve user's code even more. (That's for a next PR)

Babel-preset-gatsby

Make sure that all modules are excluded by babel-preset-env so we do not add these unnecessarily

Documentation

It should be part of the build javascript documentation that I have on my list, I'm not going to do it as part of this one.

Related Issues

Related #24866

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jun 20, 2020
@wardpeet wardpeet force-pushed the feat/polyfills branch 4 times, most recently from f670bcc to a166c3d Compare June 21, 2020 21:46
@freiksenet freiksenet added status: inkteam assigned and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jun 22, 2020
@wardpeet wardpeet force-pushed the feat/polyfills branch 3 times, most recently from 721d1b9 to 65f6fef Compare June 23, 2020 10:21
@wardpeet wardpeet mentioned this pull request Jun 23, 2020
Merged
@blainekasten
Copy link
Contributor

Looks like a bunch of tests are failing. Want me to wait to review?

@wardpeet wardpeet force-pushed the feat/polyfills branch 2 times, most recently from 2ca42c5 to 007ba39 Compare June 29, 2020 20:55
@wardpeet wardpeet force-pushed the feat/core-js-resolver branch 2 times, most recently from f61eb8c to c69e5b6 Compare June 30, 2020 11:06
@wardpeet wardpeet force-pushed the feat/polyfills branch 2 times, most recently from 85ea586 to 2e510a0 Compare June 30, 2020 11:45
@wardpeet wardpeet marked this pull request as ready for review July 1, 2020 14:52
@wardpeet wardpeet changed the base branch from feat/core-js-resolver to master July 1, 2020 14:52
@wardpeet wardpeet requested review from a team as code owners July 1, 2020 15:01
@wardpeet wardpeet force-pushed the feat/polyfills branch 4 times, most recently from 75b492b to bf5071c Compare July 3, 2020 22:39
@gatsby-cloud-staging
Copy link

gatsby-cloud-staging bot commented Jul 3, 2020 •
edited

Your pull request can be previewed in Gatsby Cloud: https://build-7975b136-847d-44a8-a481-be083f48b810.staging-previews.gtsb.io

@wardpeet wardpeet force-pushed the feat/polyfills branch 2 times, most recently from 88b6383 to aa23d41 Compare July 8, 2020 06:59
@wardpeet wardpeet requested a review from a team as a code owner July 8, 2020 06:59
@wardpeet wardpeet changed the base branch from master to feat/core-js-resolver July 8, 2020 07:35
pvdz
pvdz reviewed Jul 8, 2020
View reviewed changes
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

It looks good to me. I saw @LekoArts was going to test it so I'll leave the stamp for him.

packages/gatsby-legacy-polyfills/src/exclude.js Show resolved Hide resolved
yarn.lock Show resolved Hide resolved
packages/gatsby/cache-dir/polyfill-entry.js Show resolved Hide resolved
packages/gatsby-legacy-polyfills/.gitignore Show resolved Hide resolved
LekoArts
LekoArts reviewed Jul 8, 2020
View reviewed changes
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Tested it out on https://github.com/LekoArts/gatsby-starter-minimal-blog and https://github.com/LekoArts/gatsby-starter-specimens (develop and build + serve) and haven't seen any weird behavior/errors in the console or in the browser itself then (Haven't checked the bundle sizes though)

pvdz
pvdz previously approved these changes Jul 8, 2020
View reviewed changes
Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

I believe @LekoArts validated his tests so this is good to go.

Keep in mind that, right now, this PR is not based on master so careful with the merge button!

Base automatically changed from feat/core-js-resolver to master July 9, 2020 06:44
@wardpeet wardpeet dismissed pvdz’s stale review July 9, 2020 06:44

The base branch was changed.

LekoArts
LekoArts approved these changes Jul 9, 2020
View reviewed changes
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

👍

@wardpeet wardpeet merged commit 0b7738c into master Jul 9, 2020
@wardpeet wardpeet deleted the feat/polyfills branch July 9, 2020 07:31
@etabard
Copy link

etabard commented Jul 9, 2020

Hi ! Thanks a lot for the great work on this, this is definitely something I was looking for !
I tried the 2.24.0 version and have a small bundle size reduction (5ko gz). I now see the big polyfill separate file (26ko gz) but my main app bundle still contains a lot of core-js polyfills. I was hoping a bigger app bundle size reduction (around 20ko gz). Am I dreaming here ? :D

Capture d’écran 2020-07-09 à 11 26 58

Again, thanks for the great work here !
Have a great day

@wardpeet
Copy link
Contributor Author

wardpeet commented Jul 9, 2020

Can you send me your site please,repo so I can check it out and do more improvements.

Please invite my username or have a chat with me using, ward@gatsbyjs.com

@etabard
Copy link

etabard commented Jul 9, 2020

@wardpeet Done ! Happy to discuss it with you, I also sent you an email

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

@LekoArts LekoArts LekoArts approved these changes

@pvdz pvdz pvdz left review comments

Assignees

@blainekasten blainekasten

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@wardpeet @blainekasten @etabard @pvdz @LekoArts @freiksenet