Skip to content

Commit bbc1a4f

Browse files
clydinangular-robot[bot]
authored andcommittedJan 13, 2023
feat(@angular-devkit/build-angular): support CommonJS dependency checking in esbuild
When using the experimental esbuild-based browser application builder, input files for the build will now be checked to determine if they are non-ESM modules. This behavior is comparable to the existing behavior within the default Webpack-based browser. Warnings will now be issued for any non-ESM modules (for example, CommonJS or UMD) when script optimizations are enabled (typically production builds). ESM files can be tree- shaken and otherwise optimized in ways that CommonJS files cannot which allows for more optimized and smaller output bundle files. If any allowed dependencies are provided via the `allowedCommonJsDependencies` option, both the direct import and any deep imports of the dependency will be ignored during the checks and no diagnostic will be generated for the dependency.
1 parent 697df4f commit bbc1a4f

File tree

5 files changed

+150
-4
lines changed

5 files changed

+150
-4
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import type { Metafile, PartialMessage } from 'esbuild';
10+
11+
/**
12+
* Checks the input files of a build to determine if any of the files included
13+
* in the build are not ESM. ESM files can be tree-shaken and otherwise optimized
14+
* in ways that CommonJS and other module formats cannot. The esbuild metafile
15+
* information is used as the basis for the analysis as it contains information
16+
* for each input file including its respective format.
17+
*
18+
* If any allowed dependencies are provided via the `allowedCommonJsDependencies`
19+
* parameter, both the direct import and any deep imports will be ignored and no
20+
* diagnostic will be generated.
21+
*
22+
* If a module has been issued a diagnostic message, then all descendant modules
23+
* will not be checked. This prevents a potential massive amount of inactionable
24+
* messages since the initial module import is the cause of the problem.
25+
*
26+
* @param metafile An esbuild metafile object to check.
27+
* @param allowedCommonJsDependencies An optional list of allowed dependencies.
28+
* @returns Zero or more diagnostic messages for any non-ESM modules.
29+
*/
30+
export function checkCommonJSModules(
31+
metafile: Metafile,
32+
allowedCommonJsDependencies?: string[],
33+
): PartialMessage[] {
34+
const messages: PartialMessage[] = [];
35+
const allowedRequests = new Set(allowedCommonJsDependencies);
36+
37+
// Ignore Angular locale definitions which are currently UMD
38+
allowedRequests.add('@angular/common/locales');
39+
40+
// Find all entry points that contain code (JS/TS)
41+
const files: string[] = [];
42+
for (const { entryPoint } of Object.values(metafile.outputs)) {
43+
if (!entryPoint) {
44+
continue;
45+
}
46+
if (!isPathCode(entryPoint)) {
47+
continue;
48+
}
49+
50+
files.push(entryPoint);
51+
}
52+
53+
// Track seen files so they are only analyzed once.
54+
// Bundler runtime code is also ignored since it cannot be actionable.
55+
const seenFiles = new Set<string>(['<runtime>']);
56+
57+
// Analyze the files present by walking the import graph
58+
let currentFile: string | undefined;
59+
while ((currentFile = files.shift())) {
60+
const input = metafile.inputs[currentFile];
61+
62+
for (const imported of input.imports) {
63+
// Ignore imports that were already seen or not originally in the code (bundler injected)
64+
if (!imported.original || seenFiles.has(imported.path)) {
65+
continue;
66+
}
67+
seenFiles.add(imported.path);
68+
69+
// Only check actual code files
70+
if (!isPathCode(imported.path)) {
71+
continue;
72+
}
73+
74+
// Check if the import is ESM format and issue a diagnostic if the file is not allowed
75+
if (metafile.inputs[imported.path].format !== 'esm') {
76+
const request = imported.original;
77+
78+
let notAllowed = true;
79+
if (allowedRequests.has(request)) {
80+
notAllowed = false;
81+
} else {
82+
// Check for deep imports of allowed requests
83+
for (const allowed of allowedRequests) {
84+
if (request.startsWith(allowed + '/')) {
85+
notAllowed = false;
86+
break;
87+
}
88+
}
89+
}
90+
91+
if (notAllowed) {
92+
// Issue a diagnostic message and skip all descendants since they are also most
93+
// likely not ESM but solved by addressing this import.
94+
messages.push(createCommonJSModuleError(request, currentFile));
95+
continue;
96+
}
97+
}
98+
99+
// Add the path so that its imports can be checked
100+
files.push(imported.path);
101+
}
102+
}
103+
104+
return messages;
105+
}
106+
107+
/**
108+
* Determines if a file path has an extension that is a JavaScript or TypeScript
109+
* code file.
110+
*
111+
* @param name A path to check for code file extensions.
112+
* @returns True, if a code file path; false, otherwise.
113+
*/
114+
function isPathCode(name: string): boolean {
115+
return /\.[cm]?[jt]sx?$/.test(name);
116+
}
117+
118+
/**
119+
* Creates an esbuild diagnostic message for a given non-ESM module request.
120+
*
121+
* @param request The requested non-ESM module name.
122+
* @param importer The path of the file containing the import.
123+
* @returns A message representing the diagnostic.
124+
*/
125+
function createCommonJSModuleError(request: string, importer: string): PartialMessage {
126+
const error = {
127+
text: `Module '${request}' used by '${importer}' is not ESM`,
128+
notes: [
129+
{
130+
text:
131+
'CommonJS or AMD dependencies can cause optimization bailouts.\n' +
132+
'For more information see: https://angular.io/guide/build#configuring-commonjs-dependencies',
133+
},
134+
],
135+
};
136+
137+
return error;
138+
}

‎packages/angular_devkit/build_angular/src/builders/browser-esbuild/esbuild.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import {
1212
BuildInvalidate,
1313
BuildOptions,
1414
BuildResult,
15-
Message,
1615
OutputFile,
16+
PartialMessage,
1717
build,
1818
formatMessages,
1919
} from 'esbuild';
@@ -91,7 +91,7 @@ export async function bundle(
9191

9292
export async function logMessages(
9393
context: BuilderContext,
94-
{ errors, warnings }: { errors: Message[]; warnings: Message[] },
94+
{ errors, warnings }: { errors: PartialMessage[]; warnings: PartialMessage[] },
9595
): Promise<void> {
9696
if (warnings.length) {
9797
const warningMessages = await formatMessages(warnings, { kind: 'warning', color: true });

‎packages/angular_devkit/build_angular/src/builders/browser-esbuild/experimental-warnings.ts

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { BuilderContext } from '@angular-devkit/architect';
1010
import { Schema as BrowserBuilderOptions } from '../browser/schema';
1111

1212
const UNSUPPORTED_OPTIONS: Array<keyof BrowserBuilderOptions> = [
13-
'allowedCommonJsDependencies',
1413
'budgets',
1514
'progress',
1615
'scripts',

‎packages/angular_devkit/build_angular/src/builders/browser-esbuild/index.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
10-
import type { BuildInvalidate, BuildOptions, Metafile, OutputFile } from 'esbuild';
10+
import type { BuildInvalidate, BuildOptions, OutputFile } from 'esbuild';
1111
import assert from 'node:assert';
1212
import * as fs from 'node:fs/promises';
1313
import * as path from 'node:path';
@@ -19,6 +19,7 @@ import { FileInfo } from '../../utils/index-file/augment-index-html';
1919
import { IndexHtmlGenerator } from '../../utils/index-file/index-html-generator';
2020
import { augmentAppWithServiceWorkerEsbuild } from '../../utils/service-worker';
2121
import { getSupportedBrowsers } from '../../utils/supported-browsers';
22+
import { checkCommonJSModules } from './commonjs-checker';
2223
import { SourceFileCache, createCompilerPlugin } from './compiler-plugin';
2324
import { bundle, logMessages } from './esbuild';
2425
import { logExperimentalWarnings } from './experimental-warnings';
@@ -148,6 +149,12 @@ async function execute(
148149
outputs: { ...codeResults.metafile?.outputs, ...styleResults.metafile?.outputs },
149150
};
150151

152+
// Check metafile for CommonJS module usage if optimizing scripts
153+
if (optimizationOptions.scripts) {
154+
const messages = checkCommonJSModules(metafile, options.allowedCommonJsDependencies);
155+
await logMessages(context, { errors: [], warnings: messages });
156+
}
157+
151158
// Generate index HTML file
152159
if (indexHtmlOptions) {
153160
// Create an index HTML generator that reads from the in-memory output files

‎packages/angular_devkit/build_angular/src/builders/browser-esbuild/options.ts

+2
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ export async function normalizeOptions(
132132

133133
// Initial options to keep
134134
const {
135+
allowedCommonJsDependencies,
135136
baseHref,
136137
buildOptimizer,
137138
crossOrigin,
@@ -150,6 +151,7 @@ export async function normalizeOptions(
150151
// Return all the normalized options
151152
return {
152153
advancedOptimizations: buildOptimizer,
154+
allowedCommonJsDependencies,
153155
baseHref,
154156
cacheOptions,
155157
crossOrigin,

0 commit comments

Comments
 (0)
Please sign in to comment.