-
Notifications
You must be signed in to change notification settings - Fork 606
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
Slimming down Javascript bundle #1390
Conversation
Autotagging @bigcommerce/storefront-team @davidchin |
text: tmp.textContent || tmp.innerText, | ||
type: 'error', | ||
}); | ||
return showAlertModal(tmp.textContent || tmp.innerText); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a before and after for this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me. Pretty awesome results.
@Ubersmake This is good to go from my perspective. Can you help with some additional testing? It's a pretty broad change, so extra eyes will be helpful. |
FYI if you use Thanks for this PR it helped me with optimizing our build on a custom theme. |
* Move stencil-cli and grunt-cli to devDependencies * Use npx to run tests using local instead of global packages
In browsers that don't support objectfit, the current implementation is broken because reference to `this` is different when using arrow functions vs not.
* Update to babel 7 * Remove babel-polyfill and instead rely on useBuiltIns: 'usage' * Use @babel/preset-env * Set targets * Add webpack-bundle-analyzer to help us visualize webpack bundles * Remove transform-regenerator plugin * Bump versions to latest for all babel related packages * Use minimized jquery from distribution * Use splitChunks: all to separate node_modules into a separate file to avoid re-downloading it again when minor changes are made to app.js
* Remove sweetalert2 from global bundle * Create Foundation reveal modal to be used for error messages * Replace usage of sweetalert2 with alert modal in every place except cart, shipping estimator, and account details, which have more complex use cases * Change global/sweet-alert to return the swal instance so we can call it anywhere to get the initialized instance, rather than doing this in jquery onReady * Bump sweetalert to latest 6.x * Remove compareProducts from global bundle and add to category, brand, compare, and search pages.
* Removal of async/await results in bundle savings because webpack doesn't need to include a shim
* Remove explicit imports and instead rely on ProvidePlugin to load jQuery * Bump webpack to latest * Add noEmitOnErrors: true to production build
* We only need a small subset of jquery-migrate (to support Foundation 5.5), so pull that in directly rather than the whole library
* Webpack was fetching a secondary file for page-specific js even though there is nothing useful for several of our pages. This changes the import to a noop to avoid the secondary fetch. * Bring in babel/polyfill because the docs say to. This doesn't appear to have any effect on the bundle. * Use babel's dynamic import instead of an old plugin
* Need to bring in more from jquery-migrate to support Foundation 5.5
* Fix interface for ./globals/sweet-alert so it returns the actual sweetAlert object
* We now have two implementations of the alert modal. Sweet Alert is used on cart, shipping estimator, and account details because they have more advanced use cases. Everywhere else, we use a Foundation Reveal modal. * This commit updates the styling of the new alert modal to more closely match the Sweet Alert modal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks fine to me from a non Js background.
What?
Responding to code review on #1229 that was done post-merge
Based on recommendations from @davidchin in #1229 (comment) :
Based on #1229 (comment) :
this
is correctBased on #1229 (comment) :
Based on #1229 (comment) :
Update to babel 7
to avoid re-downloading it again when minor changes are made to app.js
Reduce usage of sweetAlert and compareProducts
except cart, shipping estimator, and account details, which
have more complex use cases
can call it anywhere to get the initialized instance, rather than
doing this in jquery onReady
brand, compare, and search pages.
Remove async/await
doesn't need to include a shim
Switch from pace to nanobar
Impact
The bundle size for the entrypoint has been reduced from 376kb to 286kb.
Before
After