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
Take dynamic dependencies into account when calculating hashes #2596
Conversation
d6021ac
to
50eb217
Compare
private computeContentHashWithDependencies(addons: Addons, options: OutputOptions): string { | ||
const hash = sha256(); | ||
|
||
// own rendered source, except for finalizer wrapping | ||
hash.update(this.getRenderedHash()); |
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.
We do need the rendered hash of the source itself here. How else would changing the source without changing any other dependency information change the hash?
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.
The rendered hash is added in visitDependencies
. I made a note to add a new test category at some point to prove this is actually the case.
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.
It might be somewhat confusing, though, that visitDependencies always visits the base chunk first. Maybe the name should be improved.
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #2435
Description
Previously, hashes were only based on the file content and the content of all static dependencies.
This will also take dynamic dependencies into account.