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

CSS with contenthash should not be revisioned #1454

Closed
Dante-101 opened this issue Apr 29, 2018 · 6 comments
Closed

CSS with contenthash should not be revisioned #1454

Dante-101 opened this issue Apr 29, 2018 · 6 comments
Labels
Bug An issue with our existing, production codebase. workbox-webpack-plugin

Comments

@Dante-101
Copy link

Dante-101 commented Apr 29, 2018

Library Affected: workbox-sw, workbox-webpack-plugin

Browser & Platform:
"all browsers"

Issue or Feature Request Description:

I am using workbox-webpack-plugin with InjectManifest plugin. I use mini-css-extract-plugin to extract my css into a separate file. For long term caching, I use contenthash in my filename but looks like workbox is not able to detect it and creates revision field for it. Thereby, re-downloading the css everytime any of my js changes.

image

@jeffposnick
Copy link
Contributor

This is similar to #1102 (comment)

I'm happy to leave this open to reinvestigate whether there's a clean solution, but the last time I looked into this, we were blocked because our plugin had no way of knowing about the hashes created by other plugins.

@scottohara
Copy link

@jeffposnick

Looking at the OPs screenshot, I note that a revision field was correctly omitted for *.js files containing a full hash in the name.

However, if using an abbreviated hash ([chunkhash:<number>]), e.g.

// webpack.config.js
output: {
  filename: "[name]-[chunkhash:6].js"   // e.g. app-a1b2c3.js
}

...a revision field is created for the *.js files as well.

This is because hash detection currently works by looking for 'known hashes' in the webpack compiliation (compilation.hash, compilation.fullHash, etc.); but doesn't account for the case when those hashes are intentionally shortened in the asset names.

If there's no easy solution to the issue underlying problem (such as webpack exposing an asset.isHashed property that Workbox can interrogate), do you think it would it be worth relaxing hash detection to a substring match of the first 5 characters (rather than an exact match) to account for these cases?

e.g.

 if (!revision || knownHashes.some((hash) => url.includes(hash.substr(0,5))) {

@jeffposnick
Copy link
Contributor

Right, so there are what I think are two different scenarios:

  • There's a hash that comes from another plugin, and we don't have a clean way of detecting that it's a hash. (This is what I think is happening with the CSS generation plugins, but it's been a while since I last investigated.)
  • There's a substring of a hash that's specified with [chunkhash:6] or the equivalent. This is something that we could allow developers to tell us about via a config option, and I've filed a feature request to track that: New webpack plugin config option to specify hash length #1458

@jeffposnick jeffposnick added Bug An issue with our existing, production codebase. P3 labels Aug 3, 2018
@jeffposnick
Copy link
Contributor

C.f. webpack/webpack#9038

@jeffposnick
Copy link
Contributor

This will be something you can accomplish in Workbox v5 by passing in a RegExp to the dontCacheBustURLsMatching config option.

Unfortunately, detecting it automatically without providing that option is not feasible for the v5 timeline.

@jeffposnick
Copy link
Contributor

This should be addressed by the current Workbox v5.0.0 alpha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An issue with our existing, production codebase. workbox-webpack-plugin
Projects
None yet
Development

No branches or pull requests

3 participants