fix: narrow TemplatePathFn pathData type by context#20987
Merged
Conversation
🦋 Changeset detectedLatest commit: 59c953f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
This PR is packaged and the instant preview is available (3dd0b3b). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@3dd0b3b
yarn add -D webpack@https://pkg.pr.new/webpack@3dd0b3b
pnpm add -D webpack@https://pkg.pr.new/webpack@3dd0b3b |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #20987 +/- ##
=======================================
Coverage 90.94% 90.94%
=======================================
Files 573 573
Lines 58940 58941 +1
Branches 15888 15888
=======================================
+ Hits 53601 53605 +4
+ Misses 5339 5336 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines webpack’s template-path callback typing by making TemplatePathFn generic over the pathData shape and then applying narrowed PathData variants in the public option callback types, so callback authors get stronger guarantees about what context fields are present.
Changes:
- Introduces
TemplatePathFn<T>plusPathDataChunk/PathDataModuleand updatesCompilation.getPath*signatures to carry the narrowerpathDatatypes through. - Retypes output/optimization/template callbacks to receive the appropriate context (
PathDataChunkfor chunk filenames,PathDataModulefor module asset filenames). - Aligns asset generator option schemas and generated declarations to treat
generator.filenameas an asset-module (module-context) filename.
Reviewed changes
Copilot reviewed 16 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
types.d.ts |
Updates generated public typings to use TemplatePathFn<T> and the narrowed PathDataChunk/PathDataModule in option callbacks and Compilation.getPath*. |
schemas/WebpackOptions.json |
Updates schema references/tsType to use the narrowed callback typings and switches asset generator filename schemas to AssetModuleFilename. |
schemas/plugins/asset/AssetResourceGeneratorOptions.check.js |
Regenerated validator output for updated schema. |
schemas/plugins/asset/AssetGeneratorOptions.check.js |
Regenerated validator output for updated schema. |
lib/TemplatedPathPlugin.js |
Adds generic JSDoc for TemplatePathFn<T> and updates interpolate signature typing. |
lib/Template.js |
Updates RenderManifestEntryTemplated.filenameTemplate type to match the new generic callback shape. |
lib/SourceMapDevToolPlugin.js |
Renames the local typedef used for append/sourceMappingURL comment typing (no behavior change). |
lib/runtime/GetChunkFilenameRuntimeModule.js |
Retypes chunk filename template plumbing to use ChunkFilenameTemplate. |
lib/runtime/AutoPublicPathRuntimeModule.js |
Small refactor to use a local chunk variable when computing the auto public path. |
lib/optimize/SplitChunksPlugin.js |
Retypes cache group filename to the chunk-context template type. |
lib/javascript/JavascriptModulesPlugin.js |
Retypes getChunkFilenameTemplate return type to ChunkFilenameTemplate. |
lib/index.js |
Adds typedef imports for PathDataChunk / PathDataModule for surfaced typing. |
lib/EvalSourceMapDevToolPlugin.js |
Removes an unused TemplatePath typedef. |
lib/css/CssModulesPlugin.js |
Retypes chunk filename template return type for CSS chunks. |
lib/Compilation.js |
Introduces PathDataChunk/PathDataModule typedefs and makes getPath* JSDoc generic over PathData. |
lib/Chunk.js |
Defines ChunkFilenameTemplate JSDoc types and uses them for filenameTemplate fields. |
lib/BannerPlugin.js |
Narrows banner interpolation pathData typing to chunk context. |
lib/asset/AssetModulesPlugin.js |
Updates typedefs to treat asset module filename options as AssetModuleFilename. |
declarations/WebpackOptions.d.ts |
Updates generated option typedefs to use TemplatePathFn<PathDataChunk> / TemplatePathFn<PathDataModule> in the appropriate places. |
.changeset/narrow-path-data-types.md |
Adds a changeset documenting the typing narrowing as a patch change. |
Files not reviewed (2)
- schemas/plugins/asset/AssetGeneratorOptions.check.js: Language not supported
- schemas/plugins/asset/AssetResourceGeneratorOptions.check.js: Language not supported
Comments suppressed due to low confidence (1)
schemas/WebpackOptions.json:217
AssetResourceGeneratorOptions.filenamenow referencesAssetModuleFilename, which (unlikeFilenameTemplate) does not enforceminLength: 1for strings. This makesfilename: ""pass schema validation forasset/resourcegenerator options. If empty filenames are not intended, consider tighteningAssetModuleFilename(or wrapping it) to keep the non-empty constraint.
"filename": {
"$ref": "#/definitions/AssetModuleFilename"
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| "filename": { | ||
| "$ref": "#/definitions/FilenameTemplate" | ||
| "$ref": "#/definitions/AssetModuleFilename" |
Make `TemplatePathFn` generic over the `PathData` shape (defaults to the existing fully-optional `PathData`) and add two narrowed variants in `Compilation.js`: - `PathDataChunk` — chunk filename / chunk asset interpolation context, where `chunk` is always provided. - `PathDataModule` — module asset interpolation context, where `module` and `chunkGraph` are always provided. Retype the public output option callbacks to use the appropriate variant, so `output.filename`, `output.chunkFilename`, `output.cssFilename`, `output.cssChunkFilename`, `output.htmlFilename`, `output.htmlChunkFilename`, and the `optimization.splitChunks.cacheGroups[*].filename` callback receive `PathDataChunk`, while `output.assetModuleFilename`, the per-module `generator.filename` / `generator.outputPath` (for `asset/resource`), and `module.parser.css.localIdentName` receive `PathDataModule`. `output.publicPath` keeps the loose `PathData` since it can be interpolated in either context. Also fix `AssetResourceGeneratorOptions.filename` and the combined `AssetGeneratorOptions.filename` schemas to reference `AssetModuleFilename` instead of `FilenameTemplate` — they were already used as module-context filenames at runtime; the schema now matches.
The TemplatePathFn narrowing is a typing-only improvement to existing public types, not a new feature; bump it as a patch.
- Switching AssetGeneratorOptions / AssetResourceGeneratorOptions filename schema refs from FilenameTemplate to AssetModuleFilename dropped the minLength: 1 string constraint. Restore it directly on AssetModuleFilename so empty strings stay invalid for both output.assetModuleFilename and the per-generator filename. - Update the CLI snapshot whose entries for module.generator.asset.filename and module.generator.asset/resource.filename now pull the AssetModuleFilename description.
e2041d4 to
935329b
Compare
The minLength: 1 added to AssetModuleFilename in the previous commit needed yarn fix:special to be re-run so the precompiled validators match the schema source.
A schema edit shipped to PR #20987 without re-running yarn fix:special; the precompiled validators went stale and CI's lint job had to catch it. Two changes so the next miss is caught at commit time instead. - Promote the regenerate-after-schema/JSDoc/runtime-template rule in AGENTS.md to a [!REQUIRED] callout, enumerate the trigger paths, and recommend yarn fix (which already chains fix:code + fix:special + fmt) as the default pre-commit step. - Add a lint-staged entry on schemas/**/*.json, tooling/generate-runtime-code.js, and tooling/generate-wasm-code.js that runs the read-only yarn lint:special (via tooling/lint-special-staged.js, which discards the file args lint-staged would otherwise append). Stale committed outputs now block the commit with the same error message CI reports.
Reverts the lint-staged wiring and tooling/lint-special-staged.js helper added in 4d5b547; the [!REQUIRED] callout in AGENTS.md from the same commit stays.
Contributor
Types CoverageCoverage after merging claude/improve-path-types-P2Qvs into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make
TemplatePathFngeneric over thePathDatashape (defaults to theexisting fully-optional
PathData) and add two narrowed variants inCompilation.js:PathDataChunk— chunk filename / chunk asset interpolation context,where
chunkis always provided.PathDataModule— module asset interpolation context, wheremoduleand
chunkGraphare always provided.Retype the public output option callbacks to use the appropriate
variant, so
output.filename,output.chunkFilename,output.cssFilename,output.cssChunkFilename,output.htmlFilename,output.htmlChunkFilename, and theoptimization.splitChunks.cacheGroups[*].filenamecallback receivePathDataChunk, whileoutput.assetModuleFilename, the per-modulegenerator.filename/generator.outputPath(forasset/resource),and
module.parser.css.localIdentNamereceivePathDataModule.output.publicPathkeeps the loosePathDatasince it can beinterpolated in either context.
Also fix
AssetResourceGeneratorOptions.filenameand the combinedAssetGeneratorOptions.filenameschemas to referenceAssetModuleFilenameinstead ofFilenameTemplate— they were alreadyused as module-context filenames at runtime; the schema now matches.