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
Allow use of contenthash along with templatehash #1065
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,16 @@ class HtmlWebpackPlugin { | |
this.options.filename = path.relative(compiler.options.output.path, filename); | ||
} | ||
|
||
// `contenthash` is introduced in webpack v4.3 | ||
// which conflicts with the plugin's existing `contenthash` method, hence | ||
// hence is renmaed to `templatehash` to avoid such conflicts | ||
const REGEXP_CONTENTHASH = /\[(?:(\w+):)?contenthash(?::([a-z]+\d*))?(?::(\d+))?\]/ig; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this could be simplified to:
No need for the verbose variable declaration and the if check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I thought of moving them both to the very top. hence assigned to a variable. Which I thought may be reusable in future. |
||
if (REGEXP_CONTENTHASH.test(this.options.filename)) { | ||
this.options.filename = this.options.filename.replace(REGEXP_CONTENTHASH, (match) => { | ||
return match.replace('contenthash', 'templatehash'); | ||
}); | ||
} | ||
|
||
// Check if webpack is running in production mode | ||
// @see https://github.com/webpack/webpack/blob/3366421f1784c449f415cda5930a8e445086f688/lib/WebpackOptionsDefaulter.js#L12-L14 | ||
const isProductionLikeMode = compiler.options.mode === 'production' || !compiler.options.mode; | ||
|
@@ -279,10 +289,11 @@ class HtmlWebpackPlugin { | |
return self.options.showErrors ? prettyError(err, compiler.context).toHtml() : 'ERROR'; | ||
}) | ||
.then(html => { | ||
const REGEXP_TEMPLATEHASH = /\[(?:(\w+):)?templatehash(?::([a-z]+\d*))?(?::(\d+))?\]/ig; | ||
// Allow to use [templatehash] as placeholder for the html-webpack-plugin name | ||
// See also https://survivejs.com/webpack/optimizing/adding-hashes-to-filenames/ | ||
// From https://github.com/webpack-contrib/extract-text-webpack-plugin/blob/8de6558e33487e7606e7cd7cb2adc2cccafef272/src/index.js#L212-L214 | ||
const finalOutputName = childCompilationOutputName.replace(/\[(?:(\w+):)?templatehash(?::([a-z]+\d*))?(?::(\d+))?\]/ig, (_, hashType, digestType, maxLength) => { | ||
const finalOutputName = childCompilationOutputName.replace(REGEXP_TEMPLATEHASH, (_, hashType, digestType, maxLength) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, maybe it's just me but I would prefer to see the regex directly here and not having to look for the verbose variable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree - it would be easier to read if we just copy the regexp into the replace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. Updated 👍 |
||
return loaderUtils.getHashDigest(Buffer.from(html, 'utf8'), hashType, digestType, parseInt(maxLength, 10)); | ||
}); | ||
// Add the evaluated html code to the webpack assets | ||
|
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.
Typo and extra hence here 😸
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.
Will fix, thanks 👍