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.

Sign up for GitHub

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

Jump to bottom

add allowNumericOnlyHash option for asset/resource #17903

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add allowNumericOnlyHash option for asset/resource #17903

wants to merge 2 commits into from

Conversation

makaria
Copy link

@makaria makaria commented Dec 28, 2023 •
edited
Loading

fixes #13628 (comment)

What kind of change does this PR introduce?

feature

Did you add tests for your changes?

yes

Does this PR introduce a breaking change?

no

What needs to be documented once your changes are merged?

the generator options for asset/resource have a new allowNumericOnlyHash option. Defaults to false. Setting it to true will opt-out from prefixed numeric hash with character a（bypass nonNumericOnlyHash when emit is false）.

Related problem

using generator.emit: false result in incorrect content hash of specific gif

Using a gif which hash using the hashing algorithm SHA-1 is: 63269829566710439434dae6582fa16f9345afbf.

In React jsx

import gif1 from '**/gif-1.gif'

...

<img src={gif1} />

In webpack.config.js

...
output: {
  ...
  hashFunction: 'sha1',
  ...
},
module: {
  ...
  rules: [
    ...
   {
      test: /\.(png|jpg|gif)(?:[?#].+)?$/,
      issuer: /\.(jsx?|tsx?)$/,
      type: 'asset/resource',
      generator: {
        filename: '[name].[contenthash:8][ext]',
        emit: false,
      },
    },
    ...
  ]
  ...
}

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Dec 28, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Member

Can you accept CLA? Also can you describe the problem deeply, because #13628 (comment) we had a bug in image-minimizer-webpack-plugin, so I don't understand the problem

@makaria
Copy link
Author

makaria commented Dec 28, 2023

i will try confirm CLA later.

as described in #13628 title, when module.rule.generator.emit is false, if output filename contains contenthash, it may result in different filename when emit is true.
in AssetGenerator
first, emit affect getTypes

webpack/lib/asset/AssetGenerator.js

Line 402 in ea3d248

if ((module.buildInfo && module.buildInfo.dataUrl) || this.emit === false) {

then in generate

webpack/lib/asset/AssetGenerator.js

Line 225 in ea3d248

generate(

because it's type is JS_TYPES, it's run into default branch

webpack/lib/asset/AssetGenerator.js

Line 240 in ea3d248

default: {

and then calculate content hash using nonNumericOnlyHash

webpack/lib/asset/AssetGenerator.js

Line 303 in ea3d248

const contentHash = nonNumericOnlyHash(

and nonNumericOnlyHash will return hash with prefixed a if the first 20 characters of the hash is all number

webpack/lib/util/nonNumericOnlyHash.js

Lines 18 to 21 in ea3d248

if (slice.match(/[^\d]/)) return slice;
return `${String.fromCharCode(
A_CODE + (parseInt(hash[0], 10) % 6)
)}${slice.slice(1)}`;

it happen because the sha1 hash of the problem gif start with 20 number characters, the solution can be:

  1. set output hashDigestLength to 40. acceptable for now. only when some file's hash contains 40 number character, then problem again
  2. set emit to true. acceptable too, but we need delete the emitted assets after each build
  3. change output hashFunction to something other than 'sha1', unacceptable for now, need purge all cdn files
  4. disable nonNumericOnlyHash with a new generator option, this is what this pr doing

it's more relevant with nonNumericOnlyHash introduced in #15289

@alexander-akait
Copy link
Member

@makaria So you need the same filename when emit is true and false? Just want to undestand this deeply

@makaria
Copy link
Author

makaria commented Dec 28, 2023

@makaria So you need the same filename when emit is true and false? Just want to undestand this deeply

yes, to be more accurately, the content hash need to be exactly the same as full hash sliced.

the assets is uploaded by backend with hashed filename, frontend calculate file content hash and replace file url with uploaded url

@alexander-akait
Copy link
Member

@makaria can you fix CI?

@makaria
Copy link
Author

makaria commented Jan 5, 2024

sorry, but i've no idea about how to fix Error: Deprecations while compiling, can you help me fix it?
btw, do you think there is other solution for such case(content hash same as full hash sliced)?I don't quite understand the issue related with nonNumericOnlyHash, maybe this PR is not compatible with the original issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
PR: conflict PR: unreviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

module.rule.generator.emit = false results in different emitted filenames
3 participants
@makaria @webpack-bot @alexander-akait