Skip to content
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

Merged

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Feb 2, 2024

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)

  • 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

CLDR-17270

  • This PR completes the ticket.

Why?

  1. it gets us closer to having a static index.html - webpack can create such files. - CLDR-17353
  2. the hash only changes if the contents actually change. The hash is based on the contents, so it's appropriate to cache them if they don't change
  3. it lets the datadog source maps work properly, because we know ahead of time what the filename is going to be.

ALLOW_MANY_COMMITS=true

- 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
@srl295 srl295 self-assigned this Feb 2, 2024
@srl295 srl295 requested a review from btangmu February 2, 2024 20:19
Copy link
Member

@btangmu btangmu left a 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",
Copy link
Member

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"?

Copy link
Member Author

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.

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

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" :-)

@srl295
Copy link
Member Author

srl295 commented Feb 6, 2024

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?

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.

@srl295 srl295 merged commit 01b8b77 into unicode-org:main Feb 6, 2024
11 checks passed
@srl295 srl295 deleted the cldr-17270/datadog-unbreak-bundle-name branch February 6, 2024 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants