Allow configuration of use of contentHash for development #234
Conversation
justin808
changed the title
ahangarha
force-pushed
the
justin808/allow-config-useContentHash
branch
from
3e53256
to
fad12e8
Compare
ahangarha
changed the title
|
@justin808 we now have reasonable tests for this feature. So if there is no other concern, this PR should be ready for merge (or maybe we freeze the features on what we have in RC1 and add this feature later in 7.1. No rush. |
| const config = require('../config') | ||
| const { isProduction } = require('../env') |
There was a problem hiding this comment.
https://github.com/shakacode/shakapacker/blob/master/package/env.js#LL13C1-L14C1
production for NODE_ENV
There was a problem hiding this comment.
May you elaborate? isProduction checked nodeEnv already and it seems that is how we check the environment in on JS side. Before also we were adding content hash if isProduction was true.
Should I do anything different?
README.md
Outdated
| @@ -216,6 +216,8 @@ Webpack intelligently includes only necessary files. In this example, the file ` | |||
|
|
|||
| `nested_entries` allows you to have webpack entry points nested in subdirectories. This defaults to false so you don't accidentally create entry points for an entire tree of files. In other words, with `nested_entries: false`, you can have your entire `source_path` used for your source (using the `source_entry_path: /`) and you place files at the top level that you want as entry points. `nested_entries: true` allows you to have entries that are in subdirectories. This is useful if you have entries that are generated, so you can have a `generated` subdirectory and easily separate generated files from the rest of your codebase. | |||
|
|
|||
| To enable/disable the usage of contentHash in any specific environment, add/modify `useContentHash` with a boolean value in `config/shakapacker.yml`. This feature is disabled for all environments except production by default. Notice that despite the possibility of enabling this option for the development environment, [it is not recommended](https://webpack.js.org/guides/build-performance/#avoid-production-specific-tooling). | |||
There was a problem hiding this comment.
not any, instead use 'in any NODE_ENV environment except production'
Add:
You may not disable the content hash for a
NODE_ENVofproductionas that would break the browser caching of assets.
you cannot override for production
package/environments/production.js
Outdated
| if (config.useContentHash === true) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`⚠️ WARNING | ||
| Setting 'useContentHash' to 'false' in production environment is not allowed! |
There was a problem hiding this comment.
for a
NODE_ENVofproduction
Production environment could mean RAILS_ENV or NODE_ENV, so that's ambiguous. NEVER be ambiguous.
| if (config.useContentHash === true) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`⚠️ WARNING | ||
| Setting 'useContentHash' to 'false' in the production environment (specified by NODE_ENV environment variable) is not allowed! | ||
| Content hashes get added to the filenames regardless of setting useContentHash in 'shakapacker.yml' to false. | ||
| `) | ||
| } |
There was a problem hiding this comment.
I just received this warning when deploying the Shakapacker v7 upgrade to production. Guessing from the warning, I would assume that the condition is incorrect. Shouldn't it state config.useContentHash === false?
There was a problem hiding this comment.
Thanks for reporting
@justin808 fixed in #320
Per #88
An experiment with adding a configuration to specify whether a contentHash is used.
Note, per webpack docs
So rather than turning contentHash back on for everybody, let's allow an easy override.
PR checklist:
Closes #88