-
Notifications
You must be signed in to change notification settings - Fork 5
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
Upgrade to Webpack v5 to allow proper Tree Shaking. #463
Conversation
Our goal is to allow tree shaking by removing formatters from formatters.js. Webpack v4 did not support this. Therefore, this PR upgrades the Theme to use v5. Note that this is a new major version of Webpack, meaning there were some breaking changes. To support v5, I had to make these changes to the webpack-config: - The keyword 'query' is no longer supported. It is called 'options' now. - Query params, such as 'name=[name].[contenthash]...', can no longer be used with loaders. Those params must be specified in 'options'. - v5 introduced 'publicPath: "auto"', but not all plugins can handle this yet. In the interim, we should set 'output.publicPath: ""'. I also upgraded grunt-webpack and the mini-css-extract-plugin to versions that better support Webpack v5. After the upgrade, I made sure that removing formatters from formatters.js tree shakes all their 3rd party dependencies. J=SLAP-827 TEST=manual Tested the following: - libPhoneNumber was not included in the bundle when nationalizedPhoneDisplay was removed from formatters.js. - Asset urls were resolved properly for a non-ML site regardless of whether or not bundle.js and bundle.css were inlined. - Asset urls were resolved properly for a ML site without inlining. - I selected a random sample of the HitchhikerJS.Formatters to ensure they worked as expected. - isMobile and isStaging from bundle.js still worked as expected. - The iFrame integration worked as expected.
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.
wow webpack 5 is only a month old too!
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.
LGTM!
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.
LGTM!
Our goal is to allow tree shaking by removing formatters from formatters.js. Webpack v4 did not support this. Therefore, this PR upgrades the Theme to use v5. Note that this is a new major version of Webpack, meaning there were some breaking changes. To support v5, I had to make these changes to the webpack-config: - The keyword 'query' is no longer supported. It is called 'options' now. - Query params, such as 'name=[name].[contenthash]...', can no longer be used with loaders. Those params must be specified in 'options'. - v5 introduced 'publicPath: "auto"', but not all plugins can handle this yet. In the interim, we should set 'output.publicPath: ""'. I also upgraded grunt-webpack and the mini-css-extract-plugin to versions that better support Webpack v5. After the upgrade, I made sure that removing formatters from formatters.js tree shakes all their 3rd party dependencies. J=SLAP-827 TEST=manual Tested the following: - libPhoneNumber was not included in the bundle when nationalizedPhoneDisplay was removed from formatters.js. - Asset urls were resolved properly for a non-ML site regardless of whether or not bundle.js and bundle.css were inlined. - Asset urls were resolved properly for a ML site without inlining. - I selected a random sample of the HitchhikerJS.Formatters to ensure they worked as expected. - isMobile and isStaging from bundle.js still worked as expected. - The iFrame integration worked as expected.
## Version 1.17.0 Version 1.17.0 of the Theme supports the new Overlay integration. It also has a number of changes to support the new, built-in Product entity type. ### Changes - A new file, static/js/formatters-custom.js was added to the Theme. This file is where any custom formatters or overrides of built-in formatters, should live. (#448) - Single language cards now use the `toLocalizedDistance` formatter to properly format entity distance. (#449) - To better support WCAG, the iFrame integration now has a `title` attribute on the iFrame. (#451) - The Theme now allows a new type of Answers integration: the Overlay. This is one of the simplest integrations offered. Additional documentation on it can be found in the Hitchhiker training. - We upgraded the Theme to use Webpack v5. This will enable proper tree-shaking of bundle.js. (#463, #470) - A new card type was added: `product-prominentimage-clickable`. The card itself is clickable. (#466) - A new formatter was added: `Formatter.hoursList`. This formatter lists the hours for an entity for every day of the week. (#467) - To improve site performance, the `defer` attribute was added to all the SDK JS assets. (#473, #474) - Small updates were made to all the product cards to support the new, built-in Product entity type. (#411, #476) - A new formatter was added for prices: `Formatter.price`. (#472) - The `JAMBO_INJECTED_DATA` environment variable now includes the label of an experience's vertical. This label is used as the default vertical name in navigation, no results, and universal section title. (#490, 493) ### Bug Fixes - The post upgrade script now works correctly when the Theme has been imported as a submodule. (#441) - The `LocationStandard` card's styling was updated to be fully compatible with IE11. (#455) - A bug was fixed that caused the Theme's inline Javascript to be minified twice. By removing this bug, Webpack builds were made considerably more performant. (#461) - Fixes were made to ensure the `View Results` button for `CollapsibleFilters` is sticky. (#468, #469) - The logic for identifying absolute URLs in the HBS templates was corrected (#475) - The thumbnail selection logic in `Formatter.image` was incorrect. It was not properly selecting the smallest thumbnail greater than the provided threshold. This has been corrected. Note that this bug had an impact on page performance as unnecessarily large images were loaded on the page. (#471) - The `relativePath` computed by Jambo is now properly passed along to the Theme's cards. (#499)
## Version 1.17.0 Version 1.17.0 of the Theme supports the new Overlay integration. It also has a number of changes to support the new, built-in Product entity type. ### Changes - A new file, static/js/formatters-custom.js was added to the Theme. This file is where any custom formatters or overrides of built-in formatters, should live. (#448) - Single language cards now use the `toLocalizedDistance` formatter to properly format entity distance. (#449) - To better support WCAG, the iFrame integration now has a `title` attribute on the iFrame. (#451) - The Theme now allows a new type of Answers integration: the Overlay. This is one of the simplest integrations offered. Additional documentation on it can be found in the Hitchhiker training. - We upgraded the Theme to use Webpack v5. This will enable proper tree-shaking of bundle.js. (#463, #470) - A new card type was added: `product-prominentimage-clickable`. The card itself is clickable. (#466) - A new formatter was added: `Formatter.hoursList`. This formatter lists the hours for an entity for every day of the week. (#467) - To improve site performance, the `defer` attribute was added to all the SDK JS assets. (#473, #474) - Small updates were made to all the product cards to support the new, built-in Product entity type. (#411, #476) - A new formatter was added for prices: `Formatter.price`. (#472) - The `JAMBO_INJECTED_DATA` environment variable now includes the label of an experience's vertical. This label is used as the default vertical name in navigation, no results, and universal section title. (#490, 493) ### Bug Fixes - The post upgrade script now works correctly when the Theme has been imported as a submodule. (#441) - The `LocationStandard` card's styling was updated to be fully compatible with IE11. (#455) - A bug was fixed that caused the Theme's inline Javascript to be minified twice. By removing this bug, Webpack builds were made considerably more performant. (#461) - Fixes were made to ensure the `View Results` button for `CollapsibleFilters` is sticky. (#468, #469) - The logic for identifying absolute URLs in the HBS templates was corrected (#475) - The thumbnail selection logic in `Formatter.image` was incorrect. It was not properly selecting the smallest thumbnail greater than the provided threshold. This has been corrected. Note that this bug had an impact on page performance as unnecessarily large images were loaded on the page. (#471) - The `relativePath` computed by Jambo is now properly passed along to the Theme's cards. (#499)
Our goal is to allow tree shaking by removing formatters from formatters.js.
Webpack v4 did not support this. Therefore, this PR upgrades the Theme to use
v5. Note that this is a new major version of Webpack, meaning there were some
breaking changes. To support v5, I had to make these changes to the
webpack-config:
used with loaders. Those params must be specified in 'options'.
yet. In the interim, we should set 'output.publicPath: ""'.
I also upgraded grunt-webpack and the mini-css-extract-plugin to versions that
better support Webpack v5.
After the upgrade, I made sure that removing formatters from formatters.js
tree shakes all their 3rd party dependencies.
J=SLAP-827
TEST=manual
Tested the following:
was removed from formatters.js.
not bundle.js and bundle.css were inlined.
worked as expected.