From 5daa0e7f5683b2857ad151c1be8d875a2a8dc4b4 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Thu, 6 Mar 2025 17:53:46 +0000 Subject: [PATCH] fix(@angular/build): exclude all entrypoints of a library from prebundling The configuration now ensures that when a package is listed for exclusion, all paths within that package including sub-paths like `@foo/bar/baz` are marked as external and not prebundled by the development server. For example, specifying `@foo/bar` in the exclude list will prevent the development server from bundling any files from the `@foo/bar` package, including its sub-paths such as `@foo/bar/baz`. This aligns with esbuild external option behaviour https://esbuild.github.io/api/#external Closes #29170 --- .../src/builders/application/execute-build.ts | 56 ++++++++++++++----- .../build/src/builders/application/options.ts | 30 +++++++++- .../src/builders/application/schema.json | 2 +- .../build/src/builders/dev-server/schema.json | 2 +- .../build-external-dependencies_spec.ts | 4 +- .../src/tools/esbuild/bundler-context.ts | 13 ++--- .../tools/esbuild/bundler-execution-result.ts | 4 +- .../tools/esbuild/external-packages-plugin.ts | 47 +++++++++++++--- .../tools/vite/plugins/id-prefix-plugin.ts | 2 +- 9 files changed, 121 insertions(+), 39 deletions(-) diff --git a/packages/angular/build/src/builders/application/execute-build.ts b/packages/angular/build/src/builders/application/execute-build.ts index d07e2a793732..72a07d8b8307 100644 --- a/packages/angular/build/src/builders/application/execute-build.ts +++ b/packages/angular/build/src/builders/application/execute-build.ts @@ -163,21 +163,49 @@ export async function executeBuild( // Analyze external imports if external options are enabled if (options.externalPackages || bundlingResult.externalConfiguration) { const { - externalConfiguration, - externalImports: { browser, server }, + externalConfiguration = [], + externalImports: { browser = [], server = [] }, } = bundlingResult; - const implicitBrowser = browser ? [...browser] : []; - const implicitServer = server ? [...server] : []; - // TODO: Implement wildcard externalConfiguration filtering - executionResult.setExternalMetadata( - externalConfiguration - ? implicitBrowser.filter((value) => !externalConfiguration.includes(value)) - : implicitBrowser, - externalConfiguration - ? implicitServer.filter((value) => !externalConfiguration.includes(value)) - : implicitServer, - externalConfiguration, - ); + // Similar to esbuild, --external:@foo/bar automatically implies --external:@foo/bar/*, + // which matches import paths like @foo/bar/baz. + // This means all paths within the @foo/bar package are also marked as external. + const exclusionsPrefixes = externalConfiguration.map((exclusion) => exclusion + '/'); + const exclusions = new Set(externalConfiguration); + const explicitExternal = new Set(); + + const isExplicitExternal = (dep: string): boolean => { + if (exclusions.has(dep)) { + return true; + } + + for (const prefix of exclusionsPrefixes) { + if (dep.startsWith(prefix)) { + return true; + } + } + + return false; + }; + + const implicitBrowser: string[] = []; + for (const dep of browser) { + if (isExplicitExternal(dep)) { + explicitExternal.add(dep); + } else { + implicitBrowser.push(dep); + } + } + + const implicitServer: string[] = []; + for (const dep of server) { + if (isExplicitExternal(dep)) { + explicitExternal.add(dep); + } else { + implicitServer.push(dep); + } + } + + executionResult.setExternalMetadata(implicitBrowser, implicitServer, [...explicitExternal]); } const { metafile, initialFiles, outputFiles } = bundlingResult; diff --git a/packages/angular/build/src/builders/application/options.ts b/packages/angular/build/src/builders/application/options.ts index 2c11e57aaed1..a58c60386c9c 100644 --- a/packages/angular/build/src/builders/application/options.ts +++ b/packages/angular/build/src/builders/application/options.ts @@ -433,7 +433,14 @@ export async function normalizeOptions( baseHref, cacheOptions, crossOrigin, - externalDependencies, + externalDependencies: normalizeExternals(externalDependencies), + externalPackages: + typeof externalPackages === 'object' + ? { + ...externalPackages, + exclude: normalizeExternals(externalPackages.exclude), + } + : externalPackages, extractLicenses, inlineStyleLanguage, jit: !aot, @@ -441,7 +448,6 @@ export async function normalizeOptions( polyfills: polyfills === undefined || Array.isArray(polyfills) ? polyfills : [polyfills], poll, progress, - externalPackages, preserveSymlinks, stylePreprocessorOptions, subresourceIntegrity, @@ -677,3 +683,23 @@ export function getLocaleBaseHref( return baseHrefSuffix !== '' ? urlJoin(baseHref, baseHrefSuffix) : undefined; } + +/** + * Normalizes an array of external dependency paths by ensuring that + * wildcard patterns (`/*`) are removed from package names. + * + * This avoids the need to handle this normalization repeatedly in our plugins, + * as esbuild already treats `--external:@foo/bar` as implicitly including + * `--external:@foo/bar/*`. By standardizing the input, we ensure consistency + * and reduce redundant checks across our plugins. + * + * @param value - An optional array of dependency paths to normalize. + * @returns A new array with wildcard patterns removed from package names, or `undefined` if input is `undefined`. + */ +function normalizeExternals(value: string[] | undefined): string[] | undefined { + if (!value) { + return undefined; + } + + return [...new Set(value.map((d) => (d.endsWith('/*') ? d.slice(0, -2) : d)))]; +} diff --git a/packages/angular/build/src/builders/application/schema.json b/packages/angular/build/src/builders/application/schema.json index a8e8e13a8016..d990e3a3cff3 100644 --- a/packages/angular/build/src/builders/application/schema.json +++ b/packages/angular/build/src/builders/application/schema.json @@ -196,7 +196,7 @@ "additionalProperties": false }, "externalDependencies": { - "description": "Exclude the listed external dependencies from being bundled into the bundle. Instead, the created bundle relies on these dependencies to be available during runtime.", + "description": "Exclude the listed external dependencies from being bundled into the bundle. Instead, the created bundle relies on these dependencies to be available during runtime. Note: `@foo/bar` marks all paths within the `@foo/bar` package as external, including sub-paths like `@foo/bar/baz`.", "type": "array", "items": { "type": "string" diff --git a/packages/angular/build/src/builders/dev-server/schema.json b/packages/angular/build/src/builders/dev-server/schema.json index c36d8614e4ea..41902e43d8d0 100644 --- a/packages/angular/build/src/builders/dev-server/schema.json +++ b/packages/angular/build/src/builders/dev-server/schema.json @@ -115,7 +115,7 @@ "type": "object", "properties": { "exclude": { - "description": "List of package imports that should not be prebundled by the development server. The packages will be bundled into the application code itself.", + "description": "List of package imports that should not be prebundled by the development server. The packages will be bundled into the application code itself. Note: specifying `@foo/bar` marks all paths within the `@foo/bar` package as excluded, including sub-paths like `@foo/bar/baz`.", "type": "array", "items": { "type": "string" } } diff --git a/packages/angular/build/src/builders/dev-server/tests/behavior/build-external-dependencies_spec.ts b/packages/angular/build/src/builders/dev-server/tests/behavior/build-external-dependencies_spec.ts index 476ea0cec47a..db6d6e6d54b6 100644 --- a/packages/angular/build/src/builders/dev-server/tests/behavior/build-external-dependencies_spec.ts +++ b/packages/angular/build/src/builders/dev-server/tests/behavior/build-external-dependencies_spec.ts @@ -48,7 +48,7 @@ describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupT it('respects import specifiers when using baseHref with trailing slash', async () => { setupTarget(harness, { - externalDependencies: ['rxjs', 'rxjs/operators'], + externalDependencies: ['rxjs'], baseHref: '/test/', }); @@ -67,7 +67,7 @@ describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupT it('respects import specifiers when using baseHref without trailing slash', async () => { setupTarget(harness, { - externalDependencies: ['rxjs', 'rxjs/operators'], + externalDependencies: ['rxjs/*'], baseHref: '/test', }); diff --git a/packages/angular/build/src/tools/esbuild/bundler-context.ts b/packages/angular/build/src/tools/esbuild/bundler-context.ts index fe28009f078b..a551ae4defba 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-context.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-context.ts @@ -359,17 +359,16 @@ export class BundlerContext { // Collect all external package names const externalImports = new Set(); for (const { imports } of Object.values(result.metafile.outputs)) { - for (const importData of imports) { + for (const { external, kind, path } of imports) { if ( - !importData.external || - SERVER_GENERATED_EXTERNALS.has(importData.path) || - (importData.kind !== 'import-statement' && - importData.kind !== 'dynamic-import' && - importData.kind !== 'require-call') + !external || + SERVER_GENERATED_EXTERNALS.has(path) || + (kind !== 'import-statement' && kind !== 'dynamic-import' && kind !== 'require-call') ) { continue; } - externalImports.add(importData.path); + + externalImports.add(path); } } diff --git a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts index 41fb62721992..1ba176287450 100644 --- a/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts +++ b/packages/angular/build/src/tools/esbuild/bundler-execution-result.ts @@ -127,9 +127,9 @@ export class ExecutionResult { setExternalMetadata( implicitBrowser: string[], implicitServer: string[], - explicit: string[] | undefined, + explicit: string[], ): void { - this.externalMetadata = { implicitBrowser, implicitServer, explicit: explicit ?? [] }; + this.externalMetadata = { implicitBrowser, implicitServer, explicit }; } get output() { diff --git a/packages/angular/build/src/tools/esbuild/external-packages-plugin.ts b/packages/angular/build/src/tools/esbuild/external-packages-plugin.ts index 03808579dd33..b968493005ac 100644 --- a/packages/angular/build/src/tools/esbuild/external-packages-plugin.ts +++ b/packages/angular/build/src/tools/esbuild/external-packages-plugin.ts @@ -19,7 +19,14 @@ const EXTERNAL_PACKAGE_RESOLUTION = Symbol('EXTERNAL_PACKAGE_RESOLUTION'); * @returns An esbuild plugin. */ export function createExternalPackagesPlugin(options?: { exclude?: string[] }): Plugin { - const exclusions = options?.exclude?.length ? new Set(options.exclude) : undefined; + const exclusions = new Set(options?.exclude); + // Similar to esbuild, --external:@foo/bar automatically implies --external:@foo/bar/*, + // which matches import paths like @foo/bar/baz. + // This means all paths within the @foo/bar package are also marked as external. + const exclusionsPrefixes = options?.exclude?.map((exclusion) => exclusion + '/') ?? []; + const seenExclusions: Set = new Set(); + const seenExternals = new Set(); + const seenNonExclusions: Set = new Set(); return { name: 'angular-external-packages', @@ -33,7 +40,7 @@ export function createExternalPackagesPlugin(options?: { exclude?: string[] }): .map(([key]) => key); // Safe to use native packages external option if no loader options or exclusions present - if (!exclusions && !loaderOptionKeys?.length) { + if (!exclusions.size && !loaderOptionKeys?.length) { build.initialOptions.packages = 'external'; return; @@ -47,10 +54,26 @@ export function createExternalPackagesPlugin(options?: { exclude?: string[] }): return null; } - if (exclusions?.has(args.path)) { + if (seenExternals.has(args.path)) { + return { external: true }; + } + + if (exclusions.has(args.path) || seenExclusions.has(args.path)) { return null; } + if (!seenNonExclusions.has(args.path)) { + for (const exclusion of exclusionsPrefixes) { + if (args.path.startsWith(exclusion)) { + seenExclusions.add(args.path); + + return null; + } + } + + seenNonExclusions.add(args.path); + } + const { importer, kind, resolveDir, namespace, pluginData = {} } = args; pluginData[EXTERNAL_PACKAGE_RESOLUTION] = true; @@ -62,11 +85,18 @@ export function createExternalPackagesPlugin(options?: { exclude?: string[] }): resolveDir, }); - // Return result if unable to resolve or explicitly marked external (externalDependencies option) - if (!result.path || result.external) { + // Return result if unable to resolve + if (!result.path) { return result; } + // Return if explicitly marked external (externalDependencies option) + if (result.external) { + seenExternals.add(args.path); + + return { external: true }; + } + // Allow customized loaders to run against configured paths regardless of location if (loaderFileExtensions.has(extname(result.path))) { return result; @@ -74,10 +104,9 @@ export function createExternalPackagesPlugin(options?: { exclude?: string[] }): // Mark paths from a node modules directory as external if (/[\\/]node_modules[\\/]/.test(result.path)) { - return { - path: args.path, - external: true, - }; + seenExternals.add(args.path); + + return { external: true }; } // Otherwise return original result diff --git a/packages/angular/build/src/tools/vite/plugins/id-prefix-plugin.ts b/packages/angular/build/src/tools/vite/plugins/id-prefix-plugin.ts index e4e4620baa1e..5e543734b863 100644 --- a/packages/angular/build/src/tools/vite/plugins/id-prefix-plugin.ts +++ b/packages/angular/build/src/tools/vite/plugins/id-prefix-plugin.ts @@ -27,7 +27,7 @@ export function createRemoveIdPrefixPlugin(externals: string[]): Plugin { return; } - const escapedExternals = externals.map(escapeRegexSpecialChars); + const escapedExternals = externals.map((e) => escapeRegexSpecialChars(e) + '(?:/.+)?'); const prefixedExternalRegex = new RegExp( `${resolvedConfig.base}${VITE_ID_PREFIX}(${escapedExternals.join('|')})`, 'g',