-
Notifications
You must be signed in to change notification settings - Fork 11.9k
fix(@angular/build): use stable worker filenames during dev server #32940
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,9 +11,10 @@ import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setu | |||||
|
|
||||||
| /** | ||||||
| * A regular expression used to check if a built worker is correctly referenced in application code. | ||||||
| * Matches both hashed (worker-ABCD1234.js) and non-hashed (worker-worker.js) filenames. | ||||||
| */ | ||||||
| const REFERENCED_WORKER_REGEXP = | ||||||
| /new Worker\(new URL\("worker-[A-Z0-9]{8}\.js", import\.meta\.url\)/; | ||||||
| /new Worker\(new URL\("worker-.+\.js", import\.meta\.url\)/; | ||||||
|
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. For improved robustness in this test, consider making the regular expression more specific. The
Suggested change
|
||||||
|
|
||||||
| describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { | ||||||
| describe('Behavior: "Bundles web worker files within application code"', () => { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -286,8 +286,9 @@ export function createCompilerPlugin( | |||||
| ); | ||||||
|
|
||||||
| // Return bundled worker file entry name to be used in the built output | ||||||
| // Match both hashed (worker-ABCD1234.js) and non-hashed (worker-<name>.js) patterns | ||||||
| const workerCodeFile = workerResult.outputFiles.find((file) => | ||||||
| /^worker-[A-Z0-9]{8}.[cm]?js$/.test(path.basename(file.path)), | ||||||
| /^worker-.+\.[cm]?js$/.test(path.basename(file.path)), | ||||||
|
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. The regex
Suggested change
|
||||||
| ); | ||||||
| assert(workerCodeFile, 'Web Worker bundled code file should always be present.'); | ||||||
| const workerCodePath = path.relative( | ||||||
|
|
@@ -763,20 +764,32 @@ function bundleWebWorker( | |||||
| workerFile: string, | ||||||
| ) { | ||||||
| try { | ||||||
| // Use content hashing only when the parent build uses hashing in entry names. | ||||||
| // During dev server (ng serve), hashing is disabled and using it for workers | ||||||
| // causes the filename hash to change on every rebuild even when the worker | ||||||
| // content hasn't changed, breaking debugging sessions. | ||||||
| const parentEntryNames = build.initialOptions.entryNames ?? ''; | ||||||
| const useHashing = parentEntryNames.includes('[hash]'); | ||||||
|
|
||||||
| return build.esbuild.buildSync({ | ||||||
| ...build.initialOptions, | ||||||
| platform: 'browser', | ||||||
| write: false, | ||||||
| bundle: true, | ||||||
| metafile: true, | ||||||
| format: 'esm', | ||||||
| entryNames: 'worker-[hash]', | ||||||
| entryNames: useHashing ? 'worker-[hash]' : 'worker-[name]', | ||||||
| entryPoints: [workerFile], | ||||||
| sourcemap: pluginOptions.sourcemap, | ||||||
| // Zone.js is not used in Web workers so no need to disable | ||||||
| supported: undefined, | ||||||
| // Plugins are not supported in sync esbuild calls | ||||||
| plugins: undefined, | ||||||
| // Splitting is not needed for workers and can cause non-deterministic output | ||||||
| splitting: false, | ||||||
| // Footer may contain build-specific data (e.g., i18n hashes) that changes | ||||||
| // between rebuilds, causing unnecessary hash changes for workers | ||||||
| footer: undefined, | ||||||
| }); | ||||||
| } catch (error) { | ||||||
| if (error && typeof error === 'object' && 'errors' in error && 'warnings' in error) { | ||||||
|
|
||||||
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.
For improved robustness in this test, consider making the regular expression more specific. The
.+pattern is quite broad. Using[\w-]+instead would restrict matches to alphanumeric characters, underscores, and hyphens, which more accurately reflects the expected worker filenames (worker-[name].jsorworker-[hash].js).