Skip to content

fix(@angular/build): strip all vite id prefixes from minified code wi…#33525

Open
FestimShyti wants to merge 1 commit into
angular:mainfrom
FestimShyti:fix-id-prefix-plugin-minified
Open

fix(@angular/build): strip all vite id prefixes from minified code wi…#33525
FestimShyti wants to merge 1 commit into
angular:mainfrom
FestimShyti:fix-id-prefix-plugin-minified

Conversation

@FestimShyti

Copy link
Copy Markdown

PR Checklist

Please check to confirm your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #33524

When the dev server serves an optimized/minified build and externalDependencies are in
effect, the angular-plugin-remove-id-prefix transform strips Vite's /@id/ prefix only from
the first external import per line. Its regex appends a greedy (?:/.+)? to each external
name, 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, and
every later /@id/ occurrence on that line is never matched. The browser then requests e.g.
http://localhost:4200/@id/@angular/router, receives the index.html fallback, and module
loading fails:

Error: Unsupported Content-Type "text/html" loading http://localhost:4200/@id/@angular/router
Modules must be served with a valid MIME type like application/javascript.

Deterministic repro of the defect:

const escapeRegexSpecialChars = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const externals = ['@angular/common', '@angular/common/http', '@angular/core', '@angular/router'].sort();
const escaped = externals.map((e) => escapeRegexSpecialChars(e) + '(?:/.+)?');
const reg = new RegExp('/@id/(' + escaped.join('|') + ')', 'g');

const code =
  'import{a}from"/@id/@angular/common/http";import{b}from"/@id/@angular/router";import{c}from"/@id/@angular/core";';

console.log(code.replace(reg, (_, name) => name));
// Actual:   import{a}from"@angular/common/http";import{b}from"/@id/@angular/router";import{c}from"/@id/@angular/core";
// Expected: import{a}from"@angular/common/http";import{b}from"@angular/router";import{c}from"@angular/core";

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 — including
on 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-line
case, 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?

  • Yes
  • No

Other information

The fix was also verified end-to-end against @angular/build 21.2.0 by patching
node_modules in a real application that previously failed to bootstrap under an optimized
ng serve — it starts correctly with the bounded regex.

#30642 reported the same symptom earlier and was closed as not reproducible — a default
(unoptimized) ng serve never triggers the bug; optimization must be enabled so the output
is single-line.

…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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant