-
Notifications
You must be signed in to change notification settings - Fork 386
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
CLDR-17270 st: don't use late cache busting for bundle.js #3492
CLDR-17270 st: don't use late cache busting for bundle.js #3492
Conversation
- instead, let webpack create a bundle name based on the content's hash, which will truly change when and only when the content changes. - webpack hook to write the file - read a manifest.json on the server side to serve that
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.
This appears to obviate the need for getCacheBustingExtension as far as js is concerned, which is great. It still leaves getCacheBustingExtension in use for css. I guess webpack still doesn't know about surveytool.css, is that right?
@@ -12,7 +32,7 @@ module.exports = (env, argv) => { | |||
cacheDirectory: path.resolve(__dirname, '../target/webpack_cache'), | |||
}, | |||
output: { | |||
filename: "bundle.js", | |||
filename: "[name].[contenthash].js", |
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.
Reference for "[name]": https://webpack.js.org/guides/output-management/
Reference for "[contenthash]": https://webpack.js.org/guides/caching/
I wish I understood better. Is "[name]" going to get replaced by "index", based on "entry: "./src/index.js"?
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.
It's actually 'main' but it doesnt matter anymore , as I load the pieces dynamically. This would let us re segment the bundle later if desired.
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.
hmmm... so where would "main" come from in this case, and where's the code change that results in loading pieces dynamically?
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.
Main is the name of the segment in webpack… We could put all of the vendor code, such as vue.js into a separate segments, since it won't change often.
The .js I added writes the segment list to a new manifest.json file and surveytool.Java reads it.
When we have a static index.html then webpack will take care of embedding the bundle automatically.
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.
Actually the docs are here https://webpack.js.org/configuration/output/#template-strings
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.
that's helpful! also here's a glossary: https://webpack.js.org/glossary/
defines, e.g., "chunk" :-)
I meant to add: we could move surveytool.css into the bundle, but not all routes use the bundle (such as some startup pages). So eventually, yes we put everything into the bundle and the only page we have is index.html This change is meant to be a little less disruptive, relatively- it doesn't change what is loaded. just how the bundle is loaded. |
changes us from using
index._GITHASH_.js
applied by cache busting function, to a statically generated webpack file/cldr-apps/dist/main.71e4a712c8f0e9d8.js
(for example)CLDR-17270
Why?
ALLOW_MANY_COMMITS=true