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

[RFC] Change bundle names to include content hashes #1037

Open
avin-kavish opened this issue Apr 2, 2020 · 9 comments
Open

[RFC] Change bundle names to include content hashes #1037

avin-kavish opened this issue Apr 2, 2020 · 9 comments
Labels
enhancement New feature or request

Comments

@avin-kavish
Copy link
Contributor

avin-kavish commented Apr 2, 2020

Context
Currently, the bundle & chunk names used across builds are the same. This can cause caching issues on the client side where browsers may serve outdated versions of the bundle. It may also cause version mismatches across chunks where a mixture of up to date and out of date chunks are executed. A common cache busting technique/practice is to include a unique hash in the file name calculated for each chunk during the build. A change in code will cause the hash to be different, thereby the file name to be different and in turn bypass the browser cache

Resulting files names would look like,

auspice.bundle.e81de2cf758ada72f306.js

Relevant Reading
https://webpack.js.org/guides/caching/#output-filenames

Possible Solution
Add [contenthash] to output.filename and output.chunkFilename in the webpack config.

@avin-kavish avin-kavish added the enhancement New feature or request label Apr 2, 2020
@HashTalmiz
Copy link

Hey, can I take this issue up?

@jameshadfield
Copy link
Member

Yes -- please do!

@HashTalmiz
Copy link

I'm facing issue while trying to install Auspice locally. Just installed miniconda, which worked fine for the conda environment, but when i enter npm install --global auspice, this shows up:
err

@jameshadfield
Copy link
Member

jameshadfield commented Apr 8, 2020

Hmm. Have not seen this particular error. To develop auspice I suggest installing from source via https://nextstrain.github.io/auspice/introduction/install#installing-from-source (apologies if you're doing this, npm install --global auspice looks to me like installing from the npm registry)

@HashTalmiz
Copy link

HashTalmiz commented Apr 8, 2020

Thanks, that worked.
Many HTML files are dependent on a single auspice.bundle.js file, only problem is that they're all scattered in many directories. I've made a way around that, but it looks like the content-hashing isn't working ;-;
This is how the output object in webpack.config.json looks like
output : { path : outputPath, filename : 'auspice.[contenthash].bundle.js', chunkFilename : 'auspice.[contenthash].chunk.[name].bundle.js', publicPath : '/dist/' },
Can you check out the changes I've made in my forked repo?

@jameshadfield
Copy link
Member

Hey @HashTalmiz -- I believe that the main bundle (./dist/auspice.bundle.js) is only referenced by a single file, index.html. This bundle then lazily loads the other chunks as needed. If you submit a PR I'd be happy to take a look 😄

jameshadfield added a commit that referenced this issue Apr 16, 2020
This adds a bundlesize check to the CI with very liberal tolerances for most of the chunks. It should identify egregious bundle size expansion and provide an easier path for manual comparison of master vs PRs. 

Currently bundlesize doesn't work with GitHub actions out-of-the-box, but this is planned for their roadmap (and they've just got $10k from Google so I'm confident it will happen). Redux-saga seems to have this working via https://github.com/redux-saga/redux-saga/pull/1952/files. 

We can revisit the settings here after the chunk hashing #1037.
jameshadfield added a commit that referenced this issue Apr 16, 2020
This adds a bundlesize check to the CI with very liberal tolerances for most of the chunks. It should identify egregious bundle size expansion and provide an easier path for manual comparison of master vs PRs.

Currently bundlesize doesn't work with GitHub actions out-of-the-box, but this is planned for their roadmap (and they've just got $10k from Google so I'm confident it will happen). Redux-saga seems to have this working via https://github.com/redux-saga/redux-saga/pull/1952/files.

We can revisit the settings here after the chunk hashing #1037.
@HashTalmiz
Copy link

CI/CD tests failed in the above mentioned PR :/ , This is my first PR on github so kindly excuse the mistakes

@jameshadfield
Copy link
Member

No problem -- I'll take a look over the coming days!

@HashTalmiz
Copy link

Thanks. Let me know what was wrong.

HashTalmiz added a commit to HashTalmiz/auspice that referenced this issue Apr 25, 2020
Removed other html page references as per the comment made here ->(nextstrain#1037 (comment))
HashTalmiz added a commit to HashTalmiz/auspice that referenced this issue Apr 30, 2020
Removed other html page references as per the comment made here ->(nextstrain#1037 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants