Skip to content

fix(@angular/build): exclude all entrypoints of a library from prebundling #29753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 42 additions & 14 deletions packages/angular/build/src/builders/application/execute-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>();

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;
Expand Down
30 changes: 28 additions & 2 deletions packages/angular/build/src/builders/application/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,15 +433,21 @@ 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,
stats: !!statsJson,
polyfills: polyfills === undefined || Array.isArray(polyfills) ? polyfills : [polyfills],
poll,
progress,
externalPackages,
preserveSymlinks,
stylePreprocessorOptions,
subresourceIntegrity,
Expand Down Expand Up @@ -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)))];
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion packages/angular/build/src/builders/dev-server/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/',
});

Expand All @@ -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',
});

Expand Down
13 changes: 6 additions & 7 deletions packages/angular/build/src/tools/esbuild/bundler-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,17 +359,16 @@ export class BundlerContext {
// Collect all external package names
const externalImports = new Set<string>();
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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(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<string> = new Set();
const seenExternals = new Set<string>();
const seenNonExclusions: Set<string> = new Set();

return {
name: 'angular-external-packages',
Expand All @@ -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;
Expand All @@ -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;

Expand All @@ -62,22 +85,28 @@ 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;
}

// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down