Skip to content

fix(native-federation): shared-package source maps — hidden flag, per-file pre-link, splice-based chunk rewrite - #1128

Merged
Aukevanoost merged 3 commits into
angular-architects:21.x.xfrom
sparlampe:fix/nf-sourcemaps-minimal
Sep 11, 2026
Merged

Aukevanoost merged 3 commits into
angular-architects:21.x.xfrom
sparlampe:fix/nf-sourcemaps-minimal

Conversation

@sparlampe

@sparlampe sparlampe commented Sep 7, 2026

Copy link
Copy Markdown

Problem

Building an app with source maps enabled leaves the maps of shared packages
(the bundles nf builds itself) broken:

  1. sourceMap.hidden is ignored — the adapter passes
    sourcemap: sourcemapOptions.scripts, so shared bundles keep their
    //# sourceMappingURL= comment even when hidden maps were requested.
  2. Post-bundle linking destroys identifier density even when composed:
    babel re-prints the minified one-line bundle, anchoring mappings per AST
    node and dropping the NAME segments the minifier recorded — DevTools
    "show original variables" misbinds locals (real values under wrong names).
  3. rewriteChunkImports re-prints every emitted file through the
    TypeScript printer — unconditionally, even files with no ./chunk-…
    import — changing the byte layout while the .map next to it stays
    untouched: every mapping in a re-printed file points at the wrong bytes.

Change

libs/native-federation/src/utils/angular-esbuild-adapter.ts:

  1. honor sourceMap.hidden:
    sourcemap: scripts && (hidden ? 'external' : true) — same mapping the
    @angular/build application builder uses.
  2. link partial declarations per input file before bundling: register
    an esbuild onLoad plugin (babel + linker over the small unminified
    fesm, with the package's own .map as inputSourceMap) — the same
    strategy @angular/build itself uses. The minifier stays the last
    map-writer and records per-identifier NAME segments. The post-bundle
    link() is kept as a fallback and now returns early when the bundle has
    no remaining ɵɵngDeclare — the common case, since the pre-link plugin
    already processed them — so esbuild's map ships byte-verbatim.

libs/native-federation-core/src/lib/utils/rewrite-chunk-imports.ts:

  1. rewrite chunk imports by splicing, never re-printing: locate the
    ./chunk-… specifier literals with es-module-lexer, replace them with a plain
    string splice, and shift the map by round-tripping mappings through
    @jridgewell/sourcemap-codec — every decoded segment keeps all its
    fields (sources, original positions, NAME entries) with only the
    generated column adjusted; names/sources/sourcesContent/
    sourceRoot are never read or written, so the shift is lossless by
    construction
    . Files without chunk imports are not written at all.
    Export surface unchanged.

@Aukevanoost

Copy link
Copy Markdown
Collaborator

thanks for this,

I verified the new rewriter directly (splice, accumulated column shift, escaped
specifiers, the no-write path, import attributes, sourceMappingURL survival) and it looks good

One part needs to come out, createPreLinkPlugin() shadows the Angular compiler plugin

esbuild runs onLoad in registration order and the first non-undefined result wins.
This plugin is registered before compilerPlugin with the same /\.[cm]?js$/ filter,
so for any file containing ɵɵngDeclare — i.e. every Angular shared package —
@angular/build's own .js handler (compiler-plugin.js:397) never runs. That handler
does more than link:

  • It already pre-links per input file with inputSourceMap:
    transformFile(request, pluginOptions.jit, sideEffects), and we pass jit: false in
    the same createCompilerPluginOptions call, so skipLinker is false. The new plugin
    is a second implementation of something that already happens.
  • With advancedOptimizations: !dev (line 322) the worker runs oxc-transform — elide
    Angular metadata, pure top-level annotations, static class member adjustment.
    Bypassing it costs bundle size and tree-shaking in production.
  • It honours sideEffects and sourceMap.vendor:
    useInputSourcemap = sourcemap && (thirdPartySourcemaps || !/node_modules/.test(filename)),
    and vendor defaults to false. Shared packages are node_modules files, so Angular
    deliberately skips their input maps. The new plugin always consumes them, and always
    emits a base64 map — even when sourceMap: false.

There's no correct position for it, which I think is the tell: moved after
compilerPlugin it becomes dead code.

The other half of that fix stands on its own. Post-bundle link() passes no sourceMaps
option to babel, so it was overwriting the file and stranding the .map — the
ɵɵngDeclare early return makes it the no-op it should always have been. Keep that, drop
the plugin.

Minimum for merge

  1. Delete createPreLinkPlugin() and its entry in the plugins array. Keep the link()
    early return.

  2. Keep the sourcemap: scripts && (hidden ? 'external' : true) line — it mirrors
    create-compiler-options.ts:34-36 exactly, which is what the esbuild-level option was
    missing.

  3. Add es-module-lexer ^1.7.0 and @jridgewell/sourcemap-codec ^1.5.0 to the
    root package.json. Right now they resolve only because webpack hoists them, and
    CI would exercise 2.3.1 while consumers install 1.x. Same convention as json5 /
    chalk / sheriff-core.

  4. Wire the test target. libs/native-federation-core/project.json has no test target,
    and CI runs nx affected -t lint test build, so the spec runs only via plugin
    inference — where @nx/jest/plugin (existing jest.config.ts) and @nx/vite/plugin
    (new vitest.config.mts) now both claim test for this project. Add an explicit
    target like native-federation-runtime has, and delete the now-dead jest.config.ts:

    "test": {
      "executor": "@nx/vitest:test",
      "outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
      "options": {
        "reportsDirectory": "../../coverage/libs/native-federation-core",
        "configFile": "libs/native-federation-core/vitest.config.mts"
      }
    }
  5. Pass the filename to the lexer: parse(sourceCode, filePath). Parse failures now abort
    the build where the TS parser was lenient, and today the error reads
    Parse error @:1:1 with no indication of which file.

Please trim the comments

The rewriter has ~35 lines of comment for ~150 lines of code, and most of it restates the
signature or re-argues the PR description. Two are worth keeping because they explain
something a reader can't see:

// dynamic import: s/e include the quotes — step inside them
// starts inside the replaced span: clamp to its start

The rest I'd drop: the JSDoc blocks on collectSpecifierEdits / rewriteChunkImports /
shiftMappings (rationale that belongs in this PR description, not the source — "the same
engine es-module-shims and Vite run on", "lossless by construction"), the SpecifierEdit
field comments, // Translate byte-offset edits into per-line column shifts above code
that does exactly that, and /** True for the .js/.mjs/.cjs files the rewriter should look at. */ on isSourceFile. Specs are fine to comment freely.

After this I'll test this in a small setup and I think it's good to go.

@sparlampe
sparlampe force-pushed the fix/nf-sourcemaps-minimal branch 2 times, most recently from ab0db92 to 69b8644 Compare September 10, 2026 20:04
@Aukevanoost

Copy link
Copy Markdown
Collaborator

Approved, will merge once the CI succeeds

…ackages

The adapter passed sourcemap: sourcemapOptions.scripts, ignoring the
hidden flag normalizeSourceMaps() returns. Map hidden to esbuild's
'external' the way @angular/build's application builder does, so shared
bundles come out comment-free while their .map files are still emitted.
…no partial declarations

The esbuild compiler plugin already links partial declarations per input
file (the adapter passes jit: false, so transformFile runs with
skipLinker false), so bundles normally leave esbuild fully linked. The
post-bundle link() pass then re-printed every bundle through babel
anyway - without a sourceMaps option, overwriting the file while the
.map beside it kept describing the old bytes, and dropping the
minifier's per-identifier NAME segments ("show original variables"
misbinds locals). Return early when no partial declaration remains: the
common case ships esbuild's source map byte-verbatim, and the babel
fallback still covers toolchains that bypass the compiler plugin. Same
check the Angular CLI uses.
…the map valid

rewriteChunkImports re-printed every emitted file through the TypeScript
printer, changing the byte layout while the source map next to it stayed
untouched — every mapping in a rewritten file pointed at the wrong bytes,
and files with no chunk imports were invalidated for nothing.

Locate the './chunk-…' specifier literals with es-module-lexer (the
engine es-module-shims and Vite run on minified bundles; already in
consumers' trees via the builder's esbuild-plugin-commonjs dependency),
rewrite them with a plain string splice, and shift the map to match by
round-tripping the mappings through @jridgewell/sourcemap-codec: every
decoded segment keeps all its fields — sources, original positions and
NAME entries — with only its generated column adjusted. Nothing else in
the map is read or written, so the shift is lossless by construction
(re-emitting through TraceMap/GenMapping instead was measurably lossy:
maybeAddMapping collapses consecutive segments sharing an original
position and resolves sources against sourceRoot). Files without chunk
imports are not written at all.

Review follow-ups: declare es-module-lexer ^1.7.0 and
@jridgewell/sourcemap-codec ^1.5.0 in the root package.json instead of
relying on hoisting, wire an explicit @nx/vitest test target for the
lib and drop the dead jest.config.ts, pass the file path to the lexer
so parse errors name the file, and trim the rewriter comments to the
load-bearing ones.
@sparlampe
sparlampe force-pushed the fix/nf-sourcemaps-minimal branch from 69b8644 to cbbd8a2 Compare September 11, 2026 08:26
@sparlampe

Copy link
Copy Markdown
Author
sourcemap:
      sourcemapOptions.scripts && (sourcemapOptions.hidden ? 'external' : true),

the types do not checkout out in above statement and were replaced by

    sourcemap: sourcemapOptions.scripts
      ? sourcemapOptions.hidden
        ? 'external'
        : true
      : false,

let me know if you prefer another solution

@Aukevanoost
Aukevanoost merged commit de51650 into angular-architects:21.x.x Sep 11, 2026
1 check passed
Aukevanoost added a commit that referenced this pull request Sep 11, 2026
… guard

esbuild's charset defaults to 'ascii', so emitted bundles carry the escape
\u0275\u0275ngDeclareComponent rather than the two characters. The early return added
in #1128 therefore fired for every shared package and link() never ran at
all, instead of only when the compiler plugin had already linked everything.

Match both spellings, and await the link calls now that the fallback is
reachable again — they were fire-and-forget, so the adapter could return
before linking finished and a linker failure surfaced as an unhandled
rejection rather than a build error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants