Skip to content

fix(@angular/build): apply file replacements to web worker entry bundles#32930

Open
maruthang wants to merge 3 commits intoangular:mainfrom
maruthang:fix-29546-worker-file-replacements
Open

fix(@angular/build): apply file replacements to web worker entry bundles#32930
maruthang wants to merge 3 commits intoangular:mainfrom
maruthang:fix-29546-worker-file-replacements

Conversation

@maruthang
Copy link
Copy Markdown
Contributor

Summary

  • Current behavior: fileReplacements entries in angular.json are silently ignored when building web workers with the esbuild application builder. Workers always bundle the original (un-replaced) files even when a production build configuration specifies replacements.
  • New behavior: File replacements are correctly applied to web worker entry bundles. Import specifiers in the worker entry file are rewritten to point to the replacement files before the synchronous esbuild sub-build runs.
  • Root cause: bundleWebWorker uses build.esbuild.buildSync() with plugins: undefined. The main build applies file replacements via Angular compiler plugin onLoad hooks, which are unavailable in synchronous esbuild builds.

The fix rewrites the worker entry file's import specifiers before bundling: each relative/absolute specifier is resolved to an absolute path, checked against the fileReplacements map, and replaced with the absolute path of the replacement file when a match is found. The modified content is passed via stdin so esbuild resolves and bundles the replacement files normally.

Closes #29546

Test plan

  • New regression test should apply file replacements inside web workers in file-replacements_spec.ts verifies the worker bundle contains the replaced value and not the original
  • Existing should replace JSON files test continues to pass
  • Build a project with a web worker that imports an environment file covered by fileReplacements and verify the production bundle uses the replacement

🤖 Generated with Claude Code

In npm/pnpm/yarn workspace setups, the package manager's list command runs
against the workspace root and may not include packages that are only declared
in a workspace member's package.json.  This caused ng update to report
"Package X is not a dependency" for packages installed in a workspace member
even though they were present and installed.

The fix reads the Angular project root's package.json directly and resolves any
declared dependencies that are resolvable from node_modules but were absent from
the package manager's output.  This restores the behaviour that was present
before the package-manager abstraction was introduced.

Closes angular#32787
The esbuild synchronous API used for worker sub-builds does not support
plugins, which are the mechanism through which file replacements are
applied in the main application bundle. As a result, fileReplacements
entries were silently ignored when bundling web workers.

Fix this by rewriting the worker entry file's import specifiers before
passing the content to buildSync. Each relative specifier is resolved to
an absolute path and compared against the fileReplacements map; matching
specifiers are replaced with the absolute path of the replacement file.
The worker is then bundled via stdin so that esbuild resolves and bundles
the replacement files normally.

Also handle the case where the worker entry file itself is replaced, and
ensure that rebuild file-tracking correctly excludes the synthetic stdin
entry added by esbuild to the metafile while still tracking the original
worker source file.

Closes angular#29546
Copy link
Copy Markdown

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

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 implements file replacement support for web workers by manually rewriting import and export specifiers in the worker entry file, addressing a limitation in esbuild's synchronous API. It also enhances the CLI update command to correctly supplement dependency maps with local packages in workspace configurations. Review feedback suggests improving the robustness of the rewriting logic by using anchored regex patterns, handling special characters in replacement paths, and supporting directory-based imports.

Comment on lines +786 to +848
function applyFileReplacementsToContent(
contents: string,
workerDir: string,
fileReplacements: Record<string, string>,
): string {
// Matches static import/export specifiers:
// import ... from 'specifier'
// export ... from 'specifier'
// import 'specifier'
// Captures the quote character (group 1) and the specifier (group 2).
const importExportRe = /\b(?:import|export)\b[^;'"]*?(['"])([^'"]+)\1/g;

// Extensions to try when resolving a specifier without an explicit extension.
const candidateExtensions = ['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs'];

let result = contents;
let match: RegExpExecArray | null;

while ((match = importExportRe.exec(contents)) !== null) {
const specifier = match[2];

// Only process relative specifiers; bare package-name imports are not file-path replacements.
if (!specifier.startsWith('.') && !path.isAbsolute(specifier)) {
continue;
}

const resolvedBase = path.isAbsolute(specifier)
? specifier
: path.join(workerDir, specifier);

let replacementPath: string | undefined;

// First check if the specifier already includes an extension and resolves directly.
const directCandidate = path.normalize(resolvedBase);
replacementPath = fileReplacements[directCandidate];

if (!replacementPath) {
// Try appending each supported extension to resolve extensionless specifiers.
for (const ext of candidateExtensions) {
const candidate = path.normalize(resolvedBase + ext);
replacementPath = fileReplacements[candidate];
if (replacementPath) {
break;
}
}
}

if (replacementPath) {
// Replace only the specifier part within the matched import/export statement.
const fullMatch = match[0];
const quote = match[1];
const newSpecifier = replacementPath.replaceAll('\\', '/');
const escapedSpecifier = specifier.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const newMatch = fullMatch.replace(
new RegExp(`${quote}${escapedSpecifier}${quote}`),
`${quote}${newSpecifier}${quote}`,
);
result = result.replace(fullMatch, newMatch);
}
}

return result;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The manual rewriting of import specifiers in the worker entry file has several limitations and potential issues:

  1. Transitive Imports: This approach only applies replacements to the entry file. Any imports within the dependency graph of the worker (i.e., files imported by the entry file) will not have replacements applied because esbuild.buildSync does not support plugins. This is a significant limitation if shared services or constants (which might import the environment) are used in the worker.
  2. Regex Robustness: The current regex can match import/export keywords inside comments or strings. Since these must be top-level statements in ESM, it's safer to anchor the regex to the start of the line.
  3. Replacement Safety: Using String.prototype.replace with a string as the second argument (line 843) can lead to incorrect output if the replacement path contains special characters like $ (e.g., in some Windows paths).
  4. Resolution Logic: The manual resolution (lines 818-831) doesn't handle directory imports (e.g., import './utils' resolving to ./utils/index.ts).

While the transitive imports issue is an architectural limitation of the synchronous worker bundling, the other issues can be mitigated by using a more robust replacement approach.

Comment on lines +796 to +845
const importExportRe = /\b(?:import|export)\b[^;'"]*?(['"])([^'"]+)\1/g;

// Extensions to try when resolving a specifier without an explicit extension.
const candidateExtensions = ['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs'];

let result = contents;
let match: RegExpExecArray | null;

while ((match = importExportRe.exec(contents)) !== null) {
const specifier = match[2];

// Only process relative specifiers; bare package-name imports are not file-path replacements.
if (!specifier.startsWith('.') && !path.isAbsolute(specifier)) {
continue;
}

const resolvedBase = path.isAbsolute(specifier)
? specifier
: path.join(workerDir, specifier);

let replacementPath: string | undefined;

// First check if the specifier already includes an extension and resolves directly.
const directCandidate = path.normalize(resolvedBase);
replacementPath = fileReplacements[directCandidate];

if (!replacementPath) {
// Try appending each supported extension to resolve extensionless specifiers.
for (const ext of candidateExtensions) {
const candidate = path.normalize(resolvedBase + ext);
replacementPath = fileReplacements[candidate];
if (replacementPath) {
break;
}
}
}

if (replacementPath) {
// Replace only the specifier part within the matched import/export statement.
const fullMatch = match[0];
const quote = match[1];
const newSpecifier = replacementPath.replaceAll('\\', '/');
const escapedSpecifier = specifier.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
const newMatch = fullMatch.replace(
new RegExp(`${quote}${escapedSpecifier}${quote}`),
`${quote}${newSpecifier}${quote}`,
);
result = result.replace(fullMatch, newMatch);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Consider using a more robust replacement logic that anchors to the start of the line and uses a callback function to avoid issues with special characters in paths. This also simplifies the loop and avoids manual string manipulation.

  // Matches static import/export specifiers at the start of a line:
  // Captures the prefix (group 1), quote (group 2), specifier (group 3), and closing quote (group 2).
  const importExportRe = /^(\s*(?:import|export)\b[^;'"\n]*?(['"]))([^'"\n]+)(\2)/gm;

  // Extensions to try when resolving a specifier without an explicit extension.
  const candidateExtensions = ['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs'];

  return contents.replace(importExportRe, (fullMatch, prefix, quote, specifier) => {
    // Only process relative specifiers; bare package-name imports are not file-path replacements.
    if (!specifier.startsWith('.') && !path.isAbsolute(specifier)) {
      return fullMatch;
    }

    const resolvedBase = path.isAbsolute(specifier)
      ? specifier
      : path.join(workerDir, specifier);

    let replacementPath: string | undefined;

    // First check if the specifier already includes an extension and resolves directly.
    const directCandidate = path.normalize(resolvedBase);
    replacementPath = fileReplacements[directCandidate];

    if (!replacementPath) {
      // Try appending each supported extension to resolve extensionless specifiers.
      for (const ext of candidateExtensions) {
        const candidate = path.normalize(resolvedBase + ext);
        replacementPath = fileReplacements[candidate];
        if (replacementPath) {
          break;
        }
      }
    }

    if (replacementPath) {
      const newSpecifier = replacementPath.replaceAll('\\', '/');
      return prefix + newSpecifier + quote;
    }

    return fullMatch;
  });

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.

File replacements are ignored for web workers

1 participant