diff --git a/defer-dependency-tree-shaking.pr-draft.md b/defer-dependency-tree-shaking.pr-draft.md new file mode 100644 index 000000000000..6fcd22ac9878 --- /dev/null +++ b/defer-dependency-tree-shaking.pr-draft.md @@ -0,0 +1,104 @@ + + +# [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) { }`, +}) +``` + +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. diff --git a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts b/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts index b0ff0593cecc..564d924b2266 100644 --- a/packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts +++ b/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( diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector.ts new file mode 100644 index 000000000000..ea079f9573c9 --- /dev/null +++ b/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; +} diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector_spec.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector_spec.ts new file mode 100644 index 000000000000..c6313dd013e5 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-detector_spec.ts @@ -0,0 +1,101 @@ +/** + * @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 { findDeferDependencyImports } from './defer-dependency-detector'; + +describe('findDeferDependencyImports', () => { + it('matches the current @angular/compiler defer resolver output', () => { + const code = ` +const App_Defer_2_DepsFn = () => [/* @ts-ignore */ + import("ngx-markdown").then(m => m.MarkdownComponent)]; +`; + + const matches = findDeferDependencyImports(code, 'app.js'); + + expect(matches.length).toBe(1); + expect(matches[0].specifier).toBe('ngx-markdown'); + expect(matches[0].symbol).toBe('MarkdownComponent'); + }); + + it('matches a default-import dependency (`m.default`)', () => { + const code = ` +const DepsFn = () => [/* @ts-ignore */ + import("clsx").then(m => m.default)]; +`; + + const matches = findDeferDependencyImports(code, 'app.js'); + + expect(matches.length).toBe(1); + expect(matches[0].symbol).toBe('default'); + }); + + it('matches every occurrence, including the ngDevMode-gated class metadata one', () => { + // Same shape shows up twice in real compiler output: once in the defer + // resolver, once in the dev-only `ɵsetClassMetadataAsync` call. + const code = ` +const App_Defer_2_DepsFn = () => [/* @ts-ignore */ + import("ngx-markdown").then(m => m.MarkdownComponent)]; +(() => { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassMetadataAsync(App, () => [/* @ts-ignore */ + import("ngx-markdown").then(m => m.MarkdownComponent)], MarkdownComponent => {}); })(); +`; + + expect(findDeferDependencyImports(code, 'app.js').length).toBe(2); + }); + + it('does not match without the @ts-ignore comment', () => { + // The shape alone isn't enough - @ts-ignore is required precisely + // because hand-written code (e.g. a Router loadComponent route) can + // have this exact shape too. + const code = `const fn = () => [import("some-lib").then(m => m.Something)];`; + + expect(findDeferDependencyImports(code, 'app.js')).toEqual([]); + }); + + it('does not match a relative specifier (Router loadComponent/loadChildren shape)', () => { + // This is the real false-positive risk: a hand-written lazy route has + // the exact same `import().then(m => m.X)` shape, and could even have + // a stray @ts-ignore above it. We should never touch this - there's no + // "whole library got pulled in" problem to fix for a relative import, + // and it's the developer's own code, not something Angular generated. + const code = ` +const routes = [{ + path: 'foo', + /* @ts-ignore */ + loadComponent: () => import('./foo.component').then(m => m.FooComponent), +}]; +`; + + expect(findDeferDependencyImports(code, 'app.js')).toEqual([]); + }); + + it('does not match an absolute specifier', () => { + const code = `const fn = () => [/* @ts-ignore */\n import("/abs/path.js").then(m => m.X)];`; + + expect(findDeferDependencyImports(code, 'app.js')).toEqual([]); + }); + + it('does not match a plain static import', () => { + const code = `import { MarkdownComponent } from 'ngx-markdown';`; + + expect(findDeferDependencyImports(code, 'app.js')).toEqual([]); + }); + + it('does not match a .then() callback that reads more than one property', () => { + const code = `const fn = () => [/* @ts-ignore */\n import("some-lib").then(m => m.a.b)];`; + + expect(findDeferDependencyImports(code, 'app.js')).toEqual([]); + }); + + it('reports offsets that span exactly the import() argument list', () => { + const code = `const fn = () => [/* @ts-ignore */\n import("some-lib").then(m => m.Foo)];`; + + const [match] = findDeferDependencyImports(code, 'app.js'); + + expect(code.slice(match.start, match.end)).toBe('"some-lib"'); + }); +}); diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-namespace.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-namespace.ts new file mode 100644 index 000000000000..0f5d08989db6 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-namespace.ts @@ -0,0 +1,46 @@ +/** + * @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 + */ + +/** + * A made-up module name esbuild lets us use. We rewrite a `@defer` import + * to point at this instead of the real package, so we can hand esbuild a + * clean, static re-export instead of a dynamic `import()`. + * + * Both the rewriter (which builds these fake import paths) and the plugin + * (which reads them back apart) use this same constant, so they can't + * accidentally get out of sync. + */ +export const DEFER_DEPENDENCY_NAMESPACE = 'angular:defer-dep'; + +/** + * Packs a package name and an export name into one string that can be + * used as an import path, e.g. `angular:defer-dep:some-lib:SomeComponent`. + * + * We can't just join the two with `:` and split on `:` later, because the + * namespace above already has a `:` in it. `encodeURIComponent` keeps + * everything safe to pull back apart in `decodeDeferDependencySpecifier` + * below, no matter what characters show up in a real package or export + * name. + */ +export function encodeDeferDependencySpecifier(specifier: string, symbol: string): string { + return `${DEFER_DEPENDENCY_NAMESPACE}:${encodeURIComponent(specifier)}:${encodeURIComponent(symbol)}`; +} + +export function decodeDeferDependencySpecifier(virtualSpecifier: string): { + specifier: string; + symbol: string; +} { + const [specifierEnc, symbolEnc] = virtualSpecifier + .slice(DEFER_DEPENDENCY_NAMESPACE.length + 1) + .split(':'); + + return { + specifier: decodeURIComponent(specifierEnc), + symbol: decodeURIComponent(symbolEnc), + }; +} diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin.ts new file mode 100644 index 000000000000..de38aabcc3bf --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin.ts @@ -0,0 +1,47 @@ +/** + * @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 type { Plugin } from 'esbuild'; +import { createVirtualModulePlugin } from '../virtual-module-plugin'; +import { + DEFER_DEPENDENCY_NAMESPACE, + decodeDeferDependencySpecifier, +} from './defer-dependency-namespace'; + +/** + * Turns a virtual specifier like `angular:defer-dep:some-lib:SomeComponent` + * into a tiny file with a single line in it: + * + * export { SomeComponent } from 'some-lib'; + * + * That's the whole trick. A plain, static re-export like this tells + * esbuild exactly which export is actually used, so it can tree-shake away + * the rest of `some-lib`. A dynamic `import()` can never do that, because + * `import()` always hands back the *entire* module, not just one export. + */ +export function createDeferDependencyPlugin(): Plugin { + return createVirtualModulePlugin({ + namespace: DEFER_DEPENDENCY_NAMESPACE, + // These virtual specifiers only ever show up inside a dynamic + // import(), never as a build entry point on their own. + entryPointOnly: false, + loadContent: (args, build) => { + const { specifier, symbol } = decodeDeferDependencySpecifier(args.path); + + return { + contents: `export { ${symbol} } from ${JSON.stringify(specifier)};`, + loader: 'js', + // Resolve the package name from the project root, same as the + // rest of the build does. This assumes one project with one + // node_modules folder, which is true for a normal Angular CLI + // app - just not something we've tested past that. + resolveDir: build.initialOptions.absWorkingDir ?? process.cwd(), + }; + }, + }); +} diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin_spec.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin_spec.ts new file mode 100644 index 000000000000..1f84565e0370 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-plugin_spec.ts @@ -0,0 +1,132 @@ +/** + * @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 * as esbuild from 'esbuild'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { createDeferDependencyPlugin } from './defer-dependency-plugin'; +import { isEsmPackage, rewriteDeferDependencyImports } from './defer-dependency-rewriter'; + +/** + * Stands in for the one line we added to compiler-plugin.ts's onLoad + * handler. This lets the test run the real rewriter and the real plugin + * together through actual esbuild bundling, without needing to spin up + * the whole Angular compiler just to get compiled output to test against. + */ +function rewriteOnLoadPlugin(): esbuild.Plugin { + return { + name: 'test-rewrite-entry', + setup(build) { + build.onLoad({ filter: /entry\.js$/ }, (args) => { + const contents = fs.readFileSync(args.path, 'utf-8'); + const rewritten = rewriteDeferDependencyImports(contents, args.path, (specifier) => + isEsmPackage(specifier, path.dirname(args.path)), + ); + + return { contents: rewritten?.code ?? contents, loader: 'js' }; + }); + }, + }; +} + +describe('createDeferDependencyPlugin', () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'defer-dep-plugin-test-')); + + // A pretend "third-party library": two exports that have nothing to + // do with each other, sideEffects: false, real ESM. This is the shape + // that should tree-shake cleanly once nothing is forcing esbuild to + // keep both of them around. + const libDir = path.join(tmpDir, 'node_modules', 'fixture-lib'); + fs.mkdirSync(libDir, { recursive: true }); + fs.writeFileSync( + path.join(libDir, 'package.json'), + JSON.stringify({ name: 'fixture-lib', type: 'module', main: 'index.js', sideEffects: false }), + ); + fs.writeFileSync( + path.join(libDir, 'index.js'), + [ + 'export class Used { greet() { return "USED_MARKER_STRING"; } }', + 'export class Unused { greet() { return "UNUSED_MARKER_STRING"; } }', + ].join('\n'), + ); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + async function bundle(entryCode: string, plugins: esbuild.Plugin[]): Promise { + fs.writeFileSync(path.join(tmpDir, 'entry.js'), entryCode); + + const result = await esbuild.build({ + absWorkingDir: tmpDir, + entryPoints: [path.join(tmpDir, 'entry.js')], + bundle: true, + write: false, + format: 'esm', + platform: 'browser', + plugins, + }); + + return result.outputFiles[0].text; + } + + const deferEntry = ` +const DepsFn = () => [/* @ts-ignore */ + import("fixture-lib").then(m => m.Used)]; +export { DepsFn }; +`; + + it('drops the unused sibling export once the rewrite plugin is applied', async () => { + const withoutPlugin = await bundle(deferEntry, []); + expect(withoutPlugin).toContain('UNUSED_MARKER_STRING'); + + const withPlugin = await bundle(deferEntry, [ + rewriteOnLoadPlugin(), + createDeferDependencyPlugin(), + ]); + expect(withPlugin).toContain('USED_MARKER_STRING'); + expect(withPlugin).not.toContain('UNUSED_MARKER_STRING'); + }); + + it('does not change output for a plain static import (nothing to rewrite)', async () => { + const staticEntry = ` +import { Used } from 'fixture-lib'; +export { Used }; +`; + const output = await bundle(staticEntry, [ + rewriteOnLoadPlugin(), + createDeferDependencyPlugin(), + ]); + + expect(output).toContain('USED_MARKER_STRING'); + expect(output).not.toContain('UNUSED_MARKER_STRING'); + }); + + it('leaves a Router-style relative lazy import alone', async () => { + fs.writeFileSync( + path.join(tmpDir, 'lazy.js'), + 'export class FooComponent { greet() { return "LAZY_ROUTE_MARKER"; } }', + ); + const routeEntry = ` +const routes = () => [ + import('./lazy.js').then(m => m.FooComponent), +]; +export { routes }; +`; + + // Should build and behave exactly as if the plugin weren't there at all. + const output = await bundle(routeEntry, [rewriteOnLoadPlugin(), createDeferDependencyPlugin()]); + expect(output).toContain('LAZY_ROUTE_MARKER'); + expect(output).not.toContain('angular:defer-dep'); + }); +}); diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter.ts new file mode 100644 index 000000000000..2d91043cc7e4 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter.ts @@ -0,0 +1,109 @@ +/** + * @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 MagicString, { type SourceMap } from 'magic-string'; +import { existsSync, readFileSync } from 'node:fs'; +import * as path from 'node:path'; +import { findDeferDependencyImports } from './defer-dependency-detector'; +import { encodeDeferDependencySpecifier } from './defer-dependency-namespace'; + +/** What a successful rewrite gives back: the new code, plus a source map for it. */ +export interface DeferDependencyRewriteResult { + code: string; + map: SourceMap; +} + +/** + * Rewrites `@defer`-generated dependency imports so they go through our + * `angular:defer-dep` virtual module instead of importing the package + * directly. That's what lets esbuild tree-shake away the parts of the + * package nothing actually uses. + * + * Returns `undefined` if there was nothing to change - either because we + * didn't find anything to rewrite, or because we found something but + * decided it wasn't worth rewriting (see `isEsmPackage` below). + */ +export function rewriteDeferDependencyImports( + code: string, + fileName: string, + isEsmPackage: (specifier: string) => boolean, +): DeferDependencyRewriteResult | undefined { + const matches = findDeferDependencyImports(code, fileName).filter((match) => + isEsmPackage(match.specifier), + ); + + if (matches.length === 0) { + return undefined; + } + + const magicString = new MagicString(code); + for (const match of matches) { + const virtualSpecifier = encodeDeferDependencySpecifier(match.specifier, match.symbol); + magicString.overwrite(match.start, match.end, JSON.stringify(virtualSpecifier)); + } + + return { + code: magicString.toString(), + map: magicString.generateMap({ source: fileName, includeContent: true, hires: true }), + }; +} + +/** + * Decides if a package is worth rewriting at all. + * + * Short version: this trick only helps for ESM packages. A CommonJS + * package gets bundled by esbuild as one single object, no matter which + * property you read off it - so rewriting the import doesn't unlock any + * tree-shaking there, it just adds an extra hop for nothing. We checked + * this against a real package (`lodash`): after rewriting, the bundle got + * *bigger*, not smaller. + * + * To find out, we walk up the folders looking for the target package's + * `package.json`, and check it for the same clues esbuild's own resolver + * looks for to know if a package ships real ESM: a `"type": "module"` + * field, a `"module"` field, or an `"import"` entry inside `"exports"`. + * + * If we can't find the package.json at all, we say "not ESM" and skip the + * rewrite. When we're not sure, doing nothing is always safe - rewriting + * something we're unsure about is not. + */ +export function isEsmPackage(specifier: string, resolveDir: string): boolean { + const packageName = specifier.startsWith('@') + ? specifier.split('/').slice(0, 2).join('/') + : specifier.split('/')[0]; + + let dir = resolveDir; + for (let i = 0; i < 10; i++) { + const packageJsonPath = path.join(dir, 'node_modules', packageName, 'package.json'); + if (existsSync(packageJsonPath)) { + let packageJson: { type?: string; module?: string; exports?: unknown }; + try { + packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf-8')); + } catch { + return false; + } + + if (packageJson.type === 'module' || packageJson.module) { + return true; + } + + // Not a full, proper check of the "exports" field - just a simple + // one. If the word "import" shows up anywhere in there, the package + // has some kind of real ESM entry point. + return !!packageJson.exports && JSON.stringify(packageJson.exports).includes('"import"'); + } + + const parentDir = path.dirname(dir); + if (parentDir === dir) { + break; + } + dir = parentDir; + } + + return false; +} diff --git a/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter_spec.ts b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter_spec.ts new file mode 100644 index 000000000000..d87f74f06f47 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/angular/defer-dependency-rewriter_spec.ts @@ -0,0 +1,148 @@ +/** + * @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 fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { isEsmPackage, rewriteDeferDependencyImports } from './defer-dependency-rewriter'; + +const DEFER_CODE = ` +const App_Defer_2_DepsFn = () => [/* @ts-ignore */ + import("ngx-markdown").then(m => m.MarkdownComponent)]; +`; + +describe('rewriteDeferDependencyImports', () => { + it('rewrites a match when the target is treated as ESM', () => { + const result = rewriteDeferDependencyImports(DEFER_CODE, 'app.js', () => true); + + expect(result).toBeDefined(); + expect(result?.code).toContain('angular:defer-dep:ngx-markdown:MarkdownComponent'); + expect(result?.code).not.toContain('import("ngx-markdown")'); + }); + + it('leaves the code untouched when the target is not treated as ESM', () => { + // This is the lodash case: rewriting a CommonJS import doesn't help, + // because esbuild bundles a CJS module as one single object no matter + // what you import from it. So we'd rather do nothing than add an + // extra step for no benefit. + const result = rewriteDeferDependencyImports(DEFER_CODE, 'app.js', () => false); + + expect(result).toBeUndefined(); + }); + + it('returns undefined when there is nothing to rewrite', () => { + const result = rewriteDeferDependencyImports('const x = 1;', 'app.js', () => true); + + expect(result).toBeUndefined(); + }); + + it('rewrites a default-import dependency', () => { + const code = `const fn = () => [/* @ts-ignore */\n import("clsx").then(m => m.default)];`; + + const result = rewriteDeferDependencyImports(code, 'app.js', () => true); + + expect(result?.code).toContain('angular:defer-dep:clsx:default'); + }); + + it('produces a source map that covers the rewritten file', () => { + const result = rewriteDeferDependencyImports(DEFER_CODE, 'app.js', () => true); + + expect(result).toBeDefined(); + const map = result?.map; + expect(map?.sources).toEqual(['app.js']); + // We asked for includeContent: true so the original source travels + // with the map, instead of devtools having to go fetch app.js on its own. + expect(map?.sourcesContent?.[0]).toBe(DEFER_CODE); + expect(map?.mappings.length).toBeGreaterThan(0); + }); + + it('does not touch unrelated lines, only the rewritten import call', () => { + const code = `const before = 1;\n${DEFER_CODE}\nconst after = 2;\n`; + + const result = rewriteDeferDependencyImports(code, 'app.js', () => true); + + expect(result?.code).toContain('const before = 1;'); + expect(result?.code).toContain('const after = 2;'); + }); +}); + +describe('isEsmPackage', () => { + let tmpDir: string; + + function writePackage(name: string, packageJson: Record): void { + const dir = path.join(tmpDir, 'node_modules', name); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify(packageJson)); + } + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'defer-dep-esm-check-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('treats a CommonJS-only package (like lodash) as not ESM', () => { + writePackage('lodash', { name: 'lodash', main: 'lodash.js' }); + + expect(isEsmPackage('lodash', tmpDir)).toBeFalse(); + }); + + it('treats "type": "module" as ESM', () => { + writePackage('a-lib', { name: 'a-lib', type: 'module', main: 'index.js' }); + + expect(isEsmPackage('a-lib', tmpDir)).toBeTrue(); + }); + + it('treats a "module" field as ESM (the ngx-markdown shape)', () => { + writePackage('ngx-markdown', { + name: 'ngx-markdown', + main: 'bundles/ngx-markdown.umd.js', + module: 'fesm2022/ngx-markdown.mjs', + sideEffects: false, + }); + + expect(isEsmPackage('ngx-markdown', tmpDir)).toBeTrue(); + }); + + it('treats an "import" condition in "exports" as ESM (the clsx shape)', () => { + writePackage('clsx', { + name: 'clsx', + main: 'dist/clsx.js', + exports: { '.': { import: './dist/clsx.mjs', default: './dist/clsx.js' } }, + }); + + expect(isEsmPackage('clsx', tmpDir)).toBeTrue(); + }); + + it('defaults to false when the package cannot be found', () => { + // If we can't find the package, play it safe and skip the rewrite + // instead of assuming it's fine. + expect(isEsmPackage('does-not-exist', tmpDir)).toBeFalse(); + }); + + it('resolves a scoped package name correctly', () => { + const dir = path.join(tmpDir, 'node_modules', '@scope', 'pkg'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'package.json'), + JSON.stringify({ name: '@scope/pkg', type: 'module' }), + ); + + expect(isEsmPackage('@scope/pkg/subpath', tmpDir)).toBeTrue(); + }); + + it('walks up parent directories to find node_modules', () => { + writePackage('a-lib', { name: 'a-lib', type: 'module' }); + const nestedDir = path.join(tmpDir, 'src', 'app'); + fs.mkdirSync(nestedDir, { recursive: true }); + + expect(isEsmPackage('a-lib', nestedDir)).toBeTrue(); + }); +}); diff --git a/packages/angular/build/src/tools/esbuild/application-code-bundle.ts b/packages/angular/build/src/tools/esbuild/application-code-bundle.ts index afb03d436d29..113fec78670e 100644 --- a/packages/angular/build/src/tools/esbuild/application-code-bundle.ts +++ b/packages/angular/build/src/tools/esbuild/application-code-bundle.ts @@ -22,6 +22,7 @@ import { import { AngularCompilationContext } from './angular/compilation-state'; import { createCompilerPlugin } from './angular/compiler-plugin'; import { ComponentStylesheetBundler } from './angular/component-stylesheets'; +import { createDeferDependencyPlugin } from './angular/defer-dependency-plugin'; import { SourceFileCache } from './angular/source-file-cache'; import { createAngularLocalizeInitWarningPlugin } from './angular-localize-init-warning-plugin'; import { BundlerOptionsFactory } from './bundler-context'; @@ -90,6 +91,9 @@ export function createBrowserCodeBundleOptions( // Component stylesheet bundler stylesheetBundler, ), + // PoC, see defer-dependency-plugin.ts. Only added here, to the main + // browser bundle - not to the server/SSR bundle further down. + createDeferDependencyPlugin(), ); if (options.plugins) {