fix(@angular-devkit/build-angular): avoid CommonJS warnings for relat…

…ive 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 82bdc9e)
angular · May 9, 2023 · ed82c83 · ed82c83
1 parent b106bc9
commit ed82c83
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 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
@@ -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/...angular/src/builders/browser-esbuild/tests/options/allowed-common-js-dependencies_spec.ts b/...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<logging.LogEntry>({
+          message: jasmine.stringMatching(/CommonJS or AMD dependencies/),
+        }),
+      );
+    });
   });
 });