From ed82c83fef1a67b4168be455b119860217267564 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Mon, 8 May 2023 13:33:33 -0400 Subject: [PATCH] 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. (cherry picked from commit 82bdc9e46094389ee1444d83df06505c6209d54c) --- .../browser-esbuild/commonjs-checker.ts | 24 +++++++++++++++++-- .../allowed-common-js-dependencies_spec.ts | 20 ++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts index 1e1591825393..24fe2a121e6b 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/commonjs-checker.ts @@ -75,8 +75,11 @@ export function checkCommonJSModules( continue; } - // Check if the import is ESM format and issue a diagnostic if the file is not allowed - if (metafile.inputs[imported.path].format !== 'esm') { + // Check if non-relative import is ESM format and issue a diagnostic if the file is not allowed + if ( + !isPotentialRelative(imported.original) && + metafile.inputs[imported.path].format !== 'esm' + ) { const request = imported.original; let notAllowed = true; @@ -119,6 +122,23 @@ function isPathCode(name: string): boolean { return /\.[cm]?[jt]sx?$/.test(name); } +/** + * Test an import module specifier to determine if the string potentially references a relative file. + * npm packages should not start with a period so if the first character is a period than it is not a + * package. While this is sufficient for the use case in the CommmonJS checker, only checking the + * first character does not definitely indicate the specifier is a relative path. + * + * @param specifier An import module specifier. + * @returns True, if specifier is potentially relative; false, otherwise. + */ +function isPotentialRelative(specifier: string): boolean { + if (specifier[0] === '.') { + return true; + } + + return false; +} + /** * Creates an esbuild diagnostic message for a given non-ESM module request. * diff --git a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts index 1270889744f3..a53a1c16b48b 100644 --- a/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts @@ -159,5 +159,25 @@ describeBuilder(buildEsbuildBrowser, BROWSER_BUILDER_INFO, (harness) => { }), ); }); + + it('should not show warning for relative imports', async () => { + await harness.appendToFile('src/main.ts', `import './abc';`); + await harness.writeFile('src/abc.ts', 'console.log("abc");'); + + harness.useTarget('build', { + ...BASE_OPTIONS, + allowedCommonJsDependencies: [], + optimization: true, + }); + + const { result, logs } = await harness.executeOnce(); + + expect(result?.success).toBe(true); + expect(logs).not.toContain( + jasmine.objectContaining({ + message: jasmine.stringMatching(/CommonJS or AMD dependencies/), + }), + ); + }); }); });