STRWEB-108 Favicons not injected into html template. #137
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
mkuklis
approved these changes
Mar 5, 2024
zburke
pushed a commit
that referenced
this pull request
Mar 7, 2024
* remove usage of speed measurer webpack plugin * log changes (cherry picked from commit e5da675)
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.
Problem
Deployed platforms are not displaying their proper favicons as required by branding configuration from
stripes-cli.Approach:
TLDR: Remove usage/plugin-wrapping of
speed-measurer-webpack-plugin... our implementation caused plugins to be seen as different types. :(Favicons are implemented in our build system via the
favicons-webpack-plugin. This ever-so-helpful plugin goes the distance by not only generating favicons for us, it also does the dirty(hacky?) work of digging into the separate instance ofhtml-webpack-pluginand injects the appropriate content into the template that you've supplied. In the case of favicons, they end up as a bunch oflink[rel="icon"]s in the page's base mark-up. In order to affect the configuration/behavior ofhtml-webpack-plugin,favicons-webpack-pluginhooks intohtml-webpack-plugin's callback system.html-webpack-pluginhas some code that lets other plugins do this - and leans on a JS Weakmap in order to ensure only a single unique set of 'hooks' exist for the plugin instance per compilation. The uniqueness can only be ensured if the objects are the all the of the same type.Our current code applies the
favicon-webpack-pluginvia its node API, within the stripes branding plugin. It has to jump through some hoops to gather the supplied branding assets. When we first addedesbuildto support transpilation and speed up build-times, we employedspeed-measurer-webpack-plugin. This was employed when we build the production webpack configuration by wrapping all of the currently existing plugins with aProxyclass, this causes it to miss the separately addedfavicons-webpack-plugin- and since it isn't wrapped, it's considered a separate process byhtml-webpack-pluginwhen it comes time to resolve its hook callbacks since the proxied webpackCompilationsare a different type from standardCompilations. The change here removes thespeed-measurer-webpack-pluginand the accompanying plugin-wrapping, so all compilations can be seen as the same type andfavicons-webpack-plugin's callback is triggered appropriately so that it can inject the favicons into the HTML Template.😅 😅 😅
favicons-webpack-pluginsets up other hooks on webpack's own hook system (where it performs actual icon generation) rather than the hooks of another plugin, so those callbacks executed fine.We could always add
speed-measurer-webpack-pluginback later, but it will have to be added to a separate place in the pipeline of our configuration.