Skip to content

Commit 82bdc9e

Browse files
clydinangular-robot[bot]
authored andcommitted
fix(@angular-devkit/build-angular): avoid CommonJS warnings for relative imports with esbuild builders
When using the esbuild-based browser application builder, CommonJS file warnings were incorrectly being issued for relative file imports. The CommonJS warnings are only intended to be generated for node module imports.
1 parent 45e98a4 commit 82bdc9e

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed

packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,11 @@ export function checkCommonJSModules(
7575
continue;
7676
}
7777

78-
// Check if the import is ESM format and issue a diagnostic if the file is not allowed
79-
if (metafile.inputs[imported.path].format !== 'esm') {
78+
// Check if non-relative import is ESM format and issue a diagnostic if the file is not allowed
79+
if (
80+
!isPotentialRelative(imported.original) &&
81+
metafile.inputs[imported.path].format !== 'esm'
82+
) {
8083
const request = imported.original;
8184

8285
let notAllowed = true;
@@ -119,6 +122,23 @@ function isPathCode(name: string): boolean {
119122
return /\.[cm]?[jt]sx?$/.test(name);
120123
}
121124

125+
/**
126+
* Test an import module specifier to determine if the string potentially references a relative file.
127+
* npm packages should not start with a period so if the first character is a period than it is not a
128+
* package. While this is sufficient for the use case in the CommmonJS checker, only checking the
129+
* first character does not definitely indicate the specifier is a relative path.
130+
*
131+
* @param specifier An import module specifier.
132+
* @returns True, if specifier is potentially relative; false, otherwise.
133+
*/
134+
function isPotentialRelative(specifier: string): boolean {
135+
if (specifier[0] === '.') {
136+
return true;
137+
}
138+
139+
return false;
140+
}
141+
122142
/**
123143
* Creates an esbuild diagnostic message for a given non-ESM module request.
124144
*

packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,5 +159,25 @@ describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => {
159159
}),
160160
);
161161
});
162+
163+
it('should not show warning for relative imports', async () => {
164+
await harness.appendToFile('src/main.ts', `import './abc';`);
165+
await harness.writeFile('src/abc.ts', 'console.log("abc");');
166+
167+
harness.useTarget('build', {
168+
...BASE_OPTIONS,
169+
allowedCommonJsDependencies: [],
170+
optimization: true,
171+
});
172+
173+
const { result, logs } = await harness.executeOnce();
174+
175+
expect(result?.success).toBe(true);
176+
expect(logs).not.toContain(
177+
jasmine.objectContaining<logging.LogEntry>({
178+
message: jasmine.stringMatching(/CommonJS or AMD dependencies/),
179+
}),
180+
);
181+
});
162182
});
163183
});

0 commit comments

Comments
 (0)