separate bundle into legacy and modern bundle #828
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tmeyer2115
reviewed
Jun 11, 2021
layouts/html.hbs
Outdated
| @@ -85,7 +85,8 @@ | |||
| }); | |||
| {{/babel}} | |||
| </script> | |||
| <script src="{{relativePath}}/bundle.js" data-webpack-inline></script> | |||
| <script src="{{relativePath}}/bundle.js" type="module" data-webpack-inline></script> | |||
| <script src="{{relativePath}}/bundle-legacy.js" nomodule></script> | |||
There was a problem hiding this comment.
I think we might need to chat with Product about this change. It has some significant upgrade implications. If somebody has forked html.hbs, their site will stop working in IE11 if they're not careful during the upgrade. The bundle.js asset no longer points to the same thing post-upgrade.
There was a problem hiding this comment.
talked to Rose and she gave the ok! people who overrode html.hbs will have to refresh their html.hbs anyways for the dev sdk branch change
This commit separates bundle.js into the polyfill-free bundle.js and polyfilled bundle-legacy.js. Both still run through the babel loader, because the esbuild minifier is set to target es5, and does not let you minify some bundles to es5 and some bundles to newer javascript. The type="module" and nomodule script loading pattern is used to dynamically load the correct bundle. This is the pattern we use for determining whether to use answers-modern or answers assets. J=SLAP-1373 TEST=manual see ~0.8s build speed improvements on yanswers and answers-atlanticsearch when I remove the core-js import from entry.js this build speedup doesn't show up for the test-site for some reason, though investigated for a while but could not figure out why see that the production modern bundle shrinks from 443KB to 313KB after removing polyfills smoke test that full page map on ie11 (browserstack), firefox, safari, chrome, and galaxy s6 chrome (browserstack)
oshi97
force-pushed
the
dev/legacy-and-modern-bundle
branch
from
c0c2214 to
f73b6a9
Compare
tmeyer2115
approved these changes
Jun 14, 2021
oshi97
added a commit
that referenced
this pull request
Jun 14, 2021
Merged
cea2aj
added a commit
that referenced
this pull request
Jun 29, 2021
Improve development build time and web performance by adding a button legacy build Create a legacy bundle build similar to #828 so that we can avoid the ponyfills and polyfills for modern web browsers. This yeids the following benefits: - Improved webpack build time by about 13% from 4.5 seconds to 4 seconds when IS_DEVELOPMENT_PREVIEW=true - Modern browsers are served a much smaller overlay-button bundle (32KB rather than the 211KB of the legacy bundle) Since specifying type="module" on the modern bundle makes it deferred, we must wait for DOMContentLoaded before trying to access `window.OverlayButtonJS` J=SLAP-1417 TEST=manual Smoke test the overlay on chrome and on IE11 on browserstack.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit separates bundle.js into the polyfill-free
bundle.js and polyfilled bundle-legacy.js. Both still
run through the babel loader, because the esbuild minifier
is set to target es5, and does not let you minify some
bundles to es5 and some bundles to newer javascript.
The type="module" and nomodule script loading pattern is used
to dynamically load the correct bundle. This is the pattern we
use for determining whether to use answers-modern or answers
assets. This shrinks bundle.js from 443KB to 313KB in prod mode
(and from 1.3MB to 705KB in dev mode). In prod mode the page speed
went from 50 -> 56, and in dev mode the page speed lighthouse scores
went from 36 -> 48.
J=SLAP-1373
TEST=manual
see ~0.7s build speed improvements on yanswers and answers-atlanticsearch
when I remove the core-js import from entry.js
this build speedup doesn't show up for the test-site for some reason, though
investigated for a while but could not figure out why
I figured that yanswers and another random repo would be better benchmarks, though
the test-site's build speed was also not hurt at all by this change
see that the production modern bundle shrinks from 443KB to 313KB after removing
polyfills
smoke test that full page map on ie11 (browserstack), firefox, safari, chrome, and
galaxy s6 chrome (browserstack)
see that bundle.js is inlined properly, and only the modern one is inlined