fix(@angular-devkit/build-angular): don't warn on transitive CommonJS…

… dependencies in AOT mode

At the moment in AOT mode if a CommonJS dependency has transitive CommonJS dependency we are issue warning for both.

With this change we align the behaviour with JIT mode, where we issue warnings only for direct CommonJS dependencies or ES dependencies which have CommonJS dependencies.

Closes #18526

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/common-js-usage-warn-plugin.ts

@@ -21,12 +21,10 @@ const STYLES_TEMPLATE_URL_REGEXP = /\.(html|svg|css|sass|less|styl|scss)$/;
 interface WebpackModule extends compilation.Module {
   name?: string;
   rawRequest?: string;
-  dependencies: unknown[];
+  dependencies: WebpackModule[];
   issuer: WebpackModule | null;
+  module: WebpackModule | null;
   userRequest?: string;
-}
-
-interface CommonJsRequireDependencyType {
   request: string;
 }
 
@@ -59,22 +57,22 @@ export class CommonJsUsageWarnPlugin {
             this.allowedDepedencies.has(this.rawRequestToPackageName(rawRequest)) ||
             rawRequest.startsWith('@angular/common/locales/')
           ) {
-          /**
-           * Skip when:
-           * - module is absolute or relative.
-           * - module is allowed even if it's a CommonJS.
-           * - module is a locale imported from '@angular/common'.
-           */
+            /**
+             * Skip when:
+             * - module is absolute or relative.
+             * - module is allowed even if it's a CommonJS.
+             * - module is a locale imported from '@angular/common'.
+             */
             continue;
           }
 
-          if (this.hasCommonJsDependencies(dependencies, true)) {
+          if (this.hasCommonJsDependencies(dependencies)) {
             // Dependency is CommonsJS or AMD.
 
             // Check if it's parent issuer is also a CommonJS dependency.
             // In case it is skip as an warning will be show for the parent CommonJS dependency.
             const parentDependencies = issuer?.issuer?.dependencies;
-            if (parentDependencies && this.hasCommonJsDependencies(parentDependencies)) {
+            if (parentDependencies && this.hasCommonJsDependencies(parentDependencies, true)) {
               continue;
             }
 
@@ -104,10 +102,10 @@ export class CommonJsUsageWarnPlugin {
     });
   }
 
-  private hasCommonJsDependencies(dependencies: unknown[], checkForStylesAndTemplatesCJS = false): boolean {
+  private hasCommonJsDependencies(dependencies: WebpackModule[], checkParentModules = false): boolean {
     for (const dep of dependencies) {
       if (dep instanceof CommonJsRequireDependency) {
-        if (checkForStylesAndTemplatesCJS && STYLES_TEMPLATE_URL_REGEXP.test((dep as CommonJsRequireDependencyType).request)) {
+        if (STYLES_TEMPLATE_URL_REGEXP.test(dep.request)) {
           // Skip in case it's a template or stylesheet
           continue;
         }
@@ -118,6 +116,10 @@ export class CommonJsUsageWarnPlugin {
       if (dep instanceof AMDDefineDependency) {
         return true;
       }
+
+      if (checkParentModules && dep.module && this.hasCommonJsDependencies(dep.module.dependencies)) {
+        return true;
+      }
     }
 
     return false;

packages/angular_devkit/build_angular/src/browser/specs/common-js-warning_spec.ts

@@ -28,20 +28,43 @@ describe('Browser Builder commonjs warning', () => {
 
   afterEach(async () => host.restore().toPromise());
 
-  it('should show warning when depending on a Common JS bundle', async () => {
-    // Add a Common JS dependency
-    host.appendToFile('src/app/app.component.ts', `
-      import 'bootstrap';
-    `);
-
-    const run = await architect.scheduleTarget(targetSpec, undefined, { logger });
-    const output = await run.result;
-    expect(output.success).toBe(true);
-    const logMsg = logs.join();
-    expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on 'bootstrap'\. CommonJS or AMD dependencies/);
-    expect(logMsg).not.toContain('jquery', 'Should not warn on transitive CommonJS packages which parent is also CommonJS.');
-    await run.stop();
-  });
+  for (const aot of [true, false]) {
+    it(`should not show warning for styles import in ${aot ? 'AOT' : 'JIT'} Mode`, async () => {
+      // Add a Common JS dependency
+      host.appendToFile('src/app/app.component.ts', `
+        import '../../test.css';
+      `);
+
+      host.writeMultipleFiles({
+        './test.css': `
+          body {
+            color: red;
+          };
+        `,
+      });
+
+      const run = await architect.scheduleTarget(targetSpec, { aot }, { logger });
+      const output = await run.result;
+      expect(output.success).toBe(true);
+      expect(logs.join()).not.toContain('WARNING');
+      await run.stop();
+    });
+
+    it(`should show warning when depending on a Common JS bundle in ${aot ? 'AOT' : 'JIT'} Mode`, async () => {
+      // Add a Common JS dependency
+      host.appendToFile('src/app/app.component.ts', `
+        import 'bootstrap';
+      `);
+
+      const run = await architect.scheduleTarget(targetSpec, { aot }, { logger });
+      const output = await run.result;
+      expect(output.success).toBe(true);
+      const logMsg = logs.join();
+      expect(logMsg).toMatch(/WARNING in.+app\.component\.ts depends on 'bootstrap'\. CommonJS or AMD dependencies/);
+      expect(logMsg).not.toContain('jquery', 'Should not warn on transitive CommonJS packages which parent is also CommonJS.');
+      await run.stop();
+    });
+  }
 
   it('should not show warning when depending on a Common JS bundle which is allowed', async () => {
     // Add a Common JS dependency