fix(@angular/build): strip all vite id prefixes from minified code wi…#33525
Open
FestimShyti wants to merge 1 commit into
Open
fix(@angular/build): strip all vite id prefixes from minified code wi…#33525FestimShyti wants to merge 1 commit into
FestimShyti wants to merge 1 commit into
Conversation
…th external dependencies The regex used by `createRemoveIdPrefixPlugin` appended a greedy `(?:/.+)?` to each external package name. On minified single-line output the first match on an external with a deep import path (e.g. `@angular/common/http`) consumed the remainder of the line, so subsequent `/@id/` specifiers were never stripped and failed in the browser with an "Unsupported Content-Type" error. The suffix is now bounded so a match cannot extend past the end of an import specifier string literal. Fixes angular#33524
There was a problem hiding this comment.
Code Review
This pull request updates the createRemoveIdPrefixPlugin Vite plugin to use a bounded regex pattern (?:/[^'"\s]+)?instead of a greedy wildcard(?:/.+)?` when matching external import paths. This prevents incorrect stripping of prefixes in minified, single-line code. Additionally, a comprehensive suite of unit tests has been added to verify this behavior under various scenarios. There are no review comments, so I have no feedback to provide.
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.
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #33524
When the dev server serves an optimized/minified build and
externalDependenciesare ineffect, the
angular-plugin-remove-id-prefixtransform strips Vite's/@id/prefix only fromthe first external import per line. Its regex appends a greedy
(?:/.+)?to each externalname, so on minified single-line output the first match on an external with a deep import path
(e.g.
@angular/common/http) consumes the remainder of the line into the capture group, andevery later
/@id/occurrence on that line is never matched. The browser then requests e.g.http://localhost:4200/@id/@angular/router, receives theindex.htmlfallback, and moduleloading fails:
Deterministic repro of the defect:
What is the new behavior?
The path suffix in the regex is bounded (
(?:/[^'"\\s]+)?) so a match can never extend past the end of an import specifier string literal. Every/@id/` occurrence is stripped — includingon minified single-line bundles — while deep import paths are still fully captured and
multi-line (unminified) behavior is unchanged.
A regression spec (
id-prefix-plugin_spec.ts) is added covering: the minified single-linecase, deep import paths, non-root
base, multi-line code, non-external@id/imports(untouched), and the empty-externals no-op. The new spec fails against the previous
implementation and passes with this change (
pnpm bazel test //packages/angular/build:test).Does this PR introduce a breaking change?
Other information
The fix was also verified end-to-end against
@angular/build21.2.0 by patchingnode_modulesin a real application that previously failed to bootstrap under an optimizedng serve— it starts correctly with the bounded regex.#30642 reported the same symptom earlier and was closed as not reproducible — a default
(unoptimized)
ng servenever triggers the bug;optimizationmust be enabled so the outputis single-line.