feat: Add support for injecting sourcemap debug ids #18947
Conversation
|
For maintainers only:
|
lib/SourceMapDevToolPlugin.js
Outdated
| .update(source) | ||
| .digest("hex"); | ||
| // The next 64 bits is a hash of the filename and sourceHash | ||
| const hash128 = `${sourceHash}${createHash("xxhash64") |
There was a problem hiding this comment.
There was a problem hiding this comment.
The same for sourceHash and respect other options, like https://webpack.js.org/configuration/output/#outputhashsalt
There was a problem hiding this comment.
Is there a reason to not just use a specific (maybe the fastest) hash here?
The code relies on getting a specific number of hex characters and if users pass a custom hashFunction, will it always return 16 hex characters?
There was a problem hiding this comment.
Is there a reason to not just use a specific (maybe the fastest) hash here?
maybe people want to change the logic depending on the options provided
There was a problem hiding this comment.
the
hashFunctioncan now be a constructor to a custom hash function. You can provide a non-crypto hash function for performance reasons.
My understanding is that users can supply a custom hashFunction if they want to change the way webpack generates file name hashes.
For Debug IDs we almost certainly don't want users to be able to configure the hash function. According to the spec, Debug ids should be a uuid derived from a deterministic hash of the file source code. The hash type isn't defined in the spec but it should be deterministic and it should have enough entropy that it is effectively unique. If we allow users to configure the hash function, the debug IDs may no longer meet the spec.
In the PR code, the 128bit debug ID is created from two xxhash64 hashes and on my machine it does this with a 300kB bundle in 2 milliseconds.
alexander-akait
left a comment
There was a problem hiding this comment.
Looks good for me, let's add
const noSources = options.devtool.includes("debugids");
here
https://github.com/webpack/webpack/blob/main/lib/WebpackOptionsApply.js#L266 and enable this option
|
I've added a test case which tests the Any pointers of how I can add this to the test case? |
|
You can add it here - You can add them here - https://github.com/webpack/webpack/tree/main/test/configCases/source-map |
|
@timfish Sorry, can you rebase? I want to merge it for the next release, thank you |
Yes, will do this in a few hours when I'm off the awful plane wifi! |
|
Right, should be good to go! |
|
Thanks |
|
I've created an issue to document this in webpack/webpack.js.org. |
What kind of change does this PR introduce?
Adds support to
SourceMapDevToolPluginfor injecting debug IDs into source files and sourcemaps as per the TC39 proposal.Did you add tests for your changes?
Not yet but will add them if this is likely to get merged!
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
I guess the
SourceMapDevToolPlugindocs need updating?Ideally I would like this option to be accessible from the top-level webpack config. I toyed with the idea of adding it to the
devtooloption (ie.hidden-source-map-debug-ids) but this option already contains a lot of different combinations of options.