feat(@angular/build): poc - shrink @defer chunks that pull in whole libraries

defer-dependency-tree-shaking.pr-draft.md

@@ -0,0 +1,104 @@
+<!--
+This is a draft PR description, kept as a plain file rather than opened as
+a real PR. Nothing on this branch is committed. Read this together with
+the diff before deciding whether to open it for real.
+-->
+
+# [DRAFT / RFC] Stop `@defer` from pulling in whole third-party libraries
+
+**Status: proof of concept, not ready to merge. Opening this mainly to ask the compiler team one question (see the bottom).**
+
+## The problem, in one example
+
+Say a component only shows up inside an `@defer` block, and it comes from a library:
+
+```ts
+import { MarkdownComponent } from 'ngx-markdown';
+
+@Component({
+  imports: [MarkdownComponent],
+  template: `@defer (on viewport) { <markdown [data]="text" /> }`,
+})
+```
+
+You'd expect the deferred chunk to contain `MarkdownComponent` and whatever it actually needs. Instead, it contains the _entire_ `ngx-markdown` package - every component, every pipe, the clipboard button, the KaTeX and Mermaid integration, all of it - even though nothing else in your app ever references those.
+
+## Why: `import()` can't ask for one export
+
+The compiler turns that defer block into something like this:
+
+```js
+import('ngx-markdown').then((m) => m.MarkdownComponent);
+```
+
+That's not a bug in the codegen - it's just what `import()` does. It's a JS operator, not syntax with a "give me just this one export" mode. It always resolves to the whole module's namespace object. So `m` is the entire `ngx-markdown` module, and nothing downstream can prove that `m.MarkdownComponent` is the only thing anyone ever reads off it. A bundler can't tree-shake what it can't prove is unused, so the rest of the package rides along.
+
+Compare that to a plain static import - `import { MarkdownComponent } from 'ngx-markdown'` - which tells the bundler exactly which binding is used, and lets it drop the rest.
+
+## Why this lives in `@angular/build`, not the compiler
+
+The compiler's job is to emit code that works everywhere Angular runs - esbuild, ng-packagr, JIT in a browser, whatever. It can't bake in "assume esbuild and rewrite the import" because that would break every other consumer of that same output. This is a decision about the _bundled_ result, and the bundler is the only place that gets to make it.
+
+## What this branch does
+
+An esbuild plugin that:
+
+1. Recognizes the shape the compiler emits (`import(specifier).then(m => m.Symbol)`, with a `@ts-ignore` comment the compiler happens to put right above it).
+2. Rewrites it to import from a synthetic virtual module instead:
+   ```js
+   import('angular:defer-dep:ngx-markdown:MarkdownComponent').then((m) => m.MarkdownComponent);
+   ```
+3. That virtual module's content is just:
+   ```js
+   export { MarkdownComponent } from 'ngx-markdown';
+   ```
+
+A static named re-export, unlike a dynamic `import()`, gives esbuild the information it needs to tree-shake the rest of the package. esbuild does the actual work here - this plugin's only job is getting a static re-export in front of it.
+
+Where it lives:
+
+- `defer-dependency-detector.ts` - just the pattern-matching, isolated on purpose (see the open question below).
+- `defer-dependency-rewriter.ts` - does the rewrite via `magic-string` (so it produces a real sourcemap), plus a guard that skips CommonJS packages (more on that below).
+- `defer-dependency-plugin.ts` - the esbuild plugin, built on the existing `createVirtualModulePlugin` helper.
+- One new line in `compiler-plugin.ts`, right before the compiled output gets cached, calling the rewriter.
+
+## Does it actually work? Numbers, not vibes
+
+**The ceiling - a library with genuinely independent exports (synthetic test, 8 unrelated classes, only 1 used):**
+
+11,988 bytes → 1,397 bytes. **88% smaller.**
+
+This is the case the bug report describes, and in that case the fix does exactly what you'd hope.
+
+**The real-world case - `ngx-markdown`, deferring `MarkdownComponent`:**
+
+59,804 bytes → 58,883 bytes. **1.5% smaller.**
+
+Much less exciting, and worth being upfront about why: `MarkdownComponent` depends on `MarkdownService`, and `MarkdownService` isn't a small, separate thing you could theoretically shake away - it's one big file that already contains the KaTeX/Mermaid/clipboard option-handling code inline, because that's how the package author wrote it. Tree-shaking _does_ correctly drop the genuinely-unrelated stuff (`ClipboardButtonComponent`, `PrismPlugin`, `MarkdownModule` - confirmed these disappear from the output), but that's a small slice of the total file. Most of the weight was never avoidable for this component, fix or no fix.
+
+Takeaway: this fix is real and it works, but how much it helps depends entirely on how a given library is structured. It'll do a lot for a component kit made of genuinely separate pieces, and not much for a library where the deferred symbol's own dependency chain already accounts for most of the bytes. `ngx-markdown` happened to be the example in the original bug report, and it's honestly not the best showcase for this - worth finding or building a better one before this goes further.
+
+**A regression we found and fixed - CommonJS packages:**
+
+Tested against `lodash` (`import("lodash").then(m => m.debounce)`). Before this fix: 73,060 bytes. First version of this fix: 73,598 bytes - _bigger_, not smaller. Turns out esbuild bundles a CommonJS module as one opaque object no matter which property you read off it afterwards, so rerouting through a static re-export doesn't unlock any tree-shaking there - it just adds an extra layer of indirection for nothing. Confirmed with a grep: both bundles contained lodash's entire export list, `debounce` or not.
+
+Fixed by checking the target package's `package.json` (`type: "module"`, a `module` field, or an `import` condition in `exports`) before rewriting anything, and leaving CommonJS packages alone entirely. Re-tested after the fix: 73,060 → 73,060 bytes, no change either way. This guard is why `defer-dependency-rewriter.ts` exists as a separate step from the plugin - it's a decision that has to happen before the rewrite, not inside esbuild's module resolution.
+
+**Default exports** work too - tested against a real package (`clsx`) by actually running the built output before and after the rewrite and confirming it computes the same result, plus a synthetic test (default export + 7 independent siblings) showing the same ~89% reduction as the named-export case.
+
+**Sourcemaps** - the rewrite runs through `magic-string`, and tracing real esbuild output back through the generated map confirms surrounding code (the untouched half of the `.then()` call) still points at the correct original line. One real gap: this rewrite's own sourcemap isn't merged into the one `javascriptTransformer.transformData` produces right after it in `compiler-plugin.ts` - fine for this PoC since the rewrite is a same-line string swap, not something to leave unresolved before merging.
+
+## What's explicitly not done here
+
+- Only wired into the main browser bundle, not the server/SSR bundle path.
+- No caching/watch-mode testing beyond reading the code and finding the right integration point (right before `typeScriptFileCache.set()`, so a cache hit returns already-rewritten content for free - untested against a real incremental rebuild).
+- Sourcemap chaining into `javascriptTransformer.transformData`'s own remapping, as mentioned above.
+- Only checked against ESM and CommonJS interop shapes, not every possible export style (re-exports under a different upstream name, e.g. `export { Foo as Bar }`, weren't tested - though the compiler always uses the name from the user's own `import` statement, so this should be transparent, just unverified).
+
+## The actual question for the compiler team
+
+The detection here is pattern-matching against `@angular/compiler`'s current output shape - specifically, an `import().then(param => param.prop)` call with a `@ts-ignore` comment sitting right above it. That comment is a real signal today (the compiler doesn't use `@ts-ignore` on that exact shape anywhere else), but it's an implementation detail of the printer, not a contract anyone promised to keep stable. It also happens to be the same comment the compiler uses for a few unrelated things elsewhere in the file, which is a little too close for comfort.
+
+**Would it be reasonable to ask for a small, dedicated marker on defer-dependency imports specifically** - a distinct comment, or something else identifiable - so a bundler doesn't have to reverse-engineer "is this a defer dependency" from what the printer happens to currently produce? The detection logic is isolated into its own file (`defer-dependency-detector.ts`) specifically so that swapping "sniff the current shape" for "read an explicit marker" would be a contained change, not a rewrite of the rewriter or the plugin.
+
+This PR is as much about surfacing that question as it is about the plugin itself.

packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts

@@ -30,6 +30,7 @@ import { LoadResultCache, createCachedLoad } from '../load-result-cache';
 import { logCumulativeDurations, profileAsync, resetCumulativeDurations } from '../profiling';
 import { AngularCompilationContext } from './compilation-state';
 import { ComponentStylesheetBundler } from './component-stylesheets';
+import { isEsmPackage, rewriteDeferDependencyImports } from './defer-dependency-rewriter';
 import { FileReferenceTracker } from './file-reference-tracker';
 import { setupJitPluginCallbacks } from './jit-plugin-callbacks';
 import { rewriteForBazel } from './rewrite-bazel-paths';
@@ -499,6 +500,29 @@ export function createCompilerPlugin(
         } else if (typeof contents === 'string' && (useTypeScriptTranspilation || isJS)) {
           // A string indicates untransformed output from the TS/NG compiler.
           // This step is unneeded when using esbuild transpilation.
+
+          // PoC: if this file has a `@defer`-generated import like
+          // `import('some-lib').then(m => m.SomeComponent)`, rewrite it so
+          // esbuild can drop the rest of `some-lib` from the deferred
+          // chunk. See defer-dependency-rewriter.ts for the details and
+          // what's still missing.
+          //
+          // We do this right here, before the cache write below, so that
+          // on the next incremental build, a cache hit already has the
+          // rewritten code and doesn't need to redo any of this work.
+          //
+          // Known gap: we don't merge our source map with the one
+          // `javascriptTransformer.transformData` creates right below.
+          // That's fine for a proof of concept - we're only swapping out a
+          // string here, nothing actually moves around - but it would need
+          // fixing before this could really be merged.
+          const rewritten = rewriteDeferDependencyImports(contents, request, (specifier) =>
+            isEsmPackage(specifier, path.dirname(request)),
+          );
+          if (rewritten) {
+            contents = rewritten.code;
+          }
+
           const sideEffects = await hasSideEffects(request);
           const instrumentForCoverage = pluginOptions.instrumentForCoverage?.(request);
           contents = await javascriptTransformer.transformData(

packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector.ts

@@ -0,0 +1,145 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import ts from 'typescript';
+
+/**
+ * A single `@defer`-generated dependency import found in a file, e.g. the
+ * `import("some-lib").then(m => m.SomeComponent)` inside a defer block's
+ * resolver function.
+ */
+export interface DeferDependencyImportMatch {
+  /** Start offset of the import()'s argument list, e.g. right after `import(`. */
+  start: number;
+
+  /** End offset of the import()'s argument list, e.g. right before `)`. */
+  end: number;
+
+  /** The module specifier as written by the user, e.g. `'some-lib'`. */
+  specifier: string;
+
+  /** The exported symbol being read off the resolved module, e.g. `SomeComponent` or `default`. */
+  symbol: string;
+}
+
+/**
+ * Looks for `@defer`-generated dependency imports in a file - the code
+ * Angular writes when a component is only used inside a `@defer` block,
+ * which looks like this:
+ *
+ *   import("some-lib").then(m => m.SomeComponent)
+ *
+ * This is just pattern matching against what `@angular/compiler` happens
+ * to produce today (see `compileDeferResolverFunction` if you want to look
+ * at the source). The compiler has never promised this exact shape will
+ * stay the same, so this logic is kept in its own file on purpose: if the
+ * compiler team ever gives us something more reliable to look for (a real
+ * marker, say), we should only need to change this one file.
+ *
+ * Why we think this match is safe enough to use:
+ * - It's rare for hand-written code to look like this. The closest
+ *   real-world example is a Router `loadComponent`/`loadChildren` route,
+ *   but those almost always use a relative path like `./foo`, so we skip
+ *   anything that isn't a plain package name.
+ * - We also require a `@ts-ignore` comment right above the import. The
+ *   compiler does use `@ts-ignore` in a few unrelated places too, but
+ *   never on this exact shape - so requiring both the shape *and* the
+ *   comment together is a pretty strong signal.
+ *
+ * Still, this is a guess, not a guarantee. See the PR description for the
+ * question we're asking the compiler team about this.
+ */
+export function findDeferDependencyImports(
+  code: string,
+  fileName: string,
+): DeferDependencyImportMatch[] {
+  const sourceFile = ts.createSourceFile(
+    fileName,
+    code,
+    ts.ScriptTarget.ES2022,
+    /* setParentNodes */ true,
+    ts.ScriptKind.JS,
+  );
+
+  const matches: DeferDependencyImportMatch[] = [];
+
+  // TypeScript actually has a built-in helper for this, `ts.isImportCall`,
+  // but it's not part of the public types for the TypeScript version this
+  // repo uses right now. So we just check the node type by hand instead.
+  function isDynamicImportCall(node: ts.CallExpression): boolean {
+    return node.expression.kind === ts.SyntaxKind.ImportKeyword;
+  }
+
+  function hasNearbyTsIgnore(node: ts.Node): boolean {
+    // Angular writes the comment like this:
+    //
+    //   [/* @ts-ignore */
+    //       import(...)]
+    //
+    // Notice the comment is on the same line as the `[` before it, not on
+    // its own line right above the import. Because of that, TypeScript
+    // doesn't count it as "belonging to" the import - so the normal way of
+    // checking for a leading comment (`ts.getLeadingCommentRanges`) misses
+    // it here. Just checking the raw text in between is simpler and works
+    // no matter how the comment is attached.
+    return code.slice(node.pos, node.getStart(sourceFile)).includes('@ts-ignore');
+  }
+
+  function visit(node: ts.Node): void {
+    if (
+      ts.isCallExpression(node) &&
+      ts.isPropertyAccessExpression(node.expression) &&
+      node.expression.name.text === 'then' &&
+      ts.isCallExpression(node.expression.expression) &&
+      isDynamicImportCall(node.expression.expression) &&
+      node.arguments.length === 1
+    ) {
+      const importCall = node.expression.expression;
+      const specifierArg = importCall.arguments[0];
+      const thenArg = node.arguments[0];
+
+      const isSimplePropertyAccessCallback =
+        (ts.isArrowFunction(thenArg) || ts.isFunctionExpression(thenArg)) &&
+        thenArg.parameters.length === 1 &&
+        ts.isIdentifier(thenArg.parameters[0].name) &&
+        !!thenArg.body &&
+        ts.isPropertyAccessExpression(thenArg.body) &&
+        ts.isIdentifier(thenArg.body.expression) &&
+        thenArg.body.expression.text === thenArg.parameters[0].name.text;
+
+      if (
+        specifierArg &&
+        ts.isStringLiteralLike(specifierArg) &&
+        isSimplePropertyAccessCallback &&
+        hasNearbyTsIgnore(node)
+      ) {
+        const specifier = specifierArg.text;
+
+        // Skip relative paths like './foo' - that's almost certainly a
+        // Router route someone wrote by hand, not something the compiler
+        // generated. It's also not a case we need to fix: a relative
+        // import points at your own file, not at an unrelated package, so
+        // there's no "whole library got pulled in" problem to solve.
+        if (!specifier.startsWith('.') && !specifier.startsWith('/')) {
+          matches.push({
+            start: importCall.arguments.pos,
+            end: importCall.arguments.end,
+            specifier,
+            symbol: (thenArg.body as ts.PropertyAccessExpression).name.text,
+          });
+        }
+      }
+    }
+
+    ts.forEachChild(node, visit);
+  }
+
+  visit(sourceFile);
+
+  return matches;
+}