fix(ivy): ngcc - correctly detect formats processed in previous runs (#32003)

Previously, `ngcc` would avoid processing a `formatPath` that a property
in `package.json` mapped to, if either the _property_ was marked as
processed or the `formatPath` (i.e. the file(s)) was processed in the
same `ngcc` run (since the `compiledFormats` set was not persisted
across runs).
This could lead in a situation where a `formatPath` would be compiled
twice (if for example properties `a` and `b` both mapped to the same
`formatPath` and one would run `ngcc` for property `a` and then `b`).

This commit fixes it by ensuring that as soon as a `formatPath` has been
processed all corresponding properties are marked as processed (which
persists across `ngcc` runs).

PR Close #32003

packages/compiler-cli/ngcc/src/main.ts

@@ -18,7 +18,7 @@ import {ConsoleLogger, LogLevel} from './logging/console_logger';
 import {Logger} from './logging/logger';
 import {hasBeenProcessed, markAsProcessed} from './packages/build_marker';
 import {NgccConfiguration} from './packages/configuration';
-import {EntryPoint, EntryPointJsonProperty, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point';
+import {EntryPoint, EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point';
 import {makeEntryPointBundle} from './packages/entry_point_bundle';
 import {Transformer} from './packages/transformer';
 import {PathMappings} from './utils';
@@ -107,6 +107,7 @@ export function mainNgcc(
     const compiledFormats = new Set<string>();
     const entryPointPackageJson = entryPoint.packageJson;
     const entryPointPackageJsonPath = fileSystem.resolve(entryPoint.path, 'package.json');
+    const pathToPropsMap = getFormatPathToPropertiesMap(entryPointPackageJson);
 
     let processDts = !hasBeenProcessed(entryPointPackageJson, 'typings');
 
@@ -128,27 +129,18 @@ export function mainNgcc(
       }
 
 
-      // We don't break if this if statement fails because we still want to mark
-      // the property as processed even if its underlying format has been built already.
       if (!compiledFormats.has(formatPath)) {
         const bundle = makeEntryPointBundle(
             fileSystem, entryPoint, formatPath, isCore, property, format, processDts, pathMappings,
             true);
+
         if (bundle) {
           logger.info(`Compiling ${entryPoint.name} : ${property} as ${format}`);
           const transformedFiles = transformer.transform(bundle);
           fileWriter.writeBundle(entryPoint, bundle, transformedFiles);
           compiledFormats.add(formatPath);
-        } else {
-          logger.warn(
-              `Skipping ${entryPoint.name} : ${format} (no valid entry point file for this format).`);
-        }
-      }
 
-      // Either this format was just compiled or its underlying format was compiled because of a
+          const propsToMarkAsProcessed = pathToPropsMap.get(formatPath) !;
-      // previous property.
-      if (compiledFormats.has(formatPath)) {
-        const propsToMarkAsProcessed = [property];
           if (processDts) {
             propsToMarkAsProcessed.push('typings');
             processDts = false;
@@ -156,6 +148,10 @@ export function mainNgcc(
 
           markAsProcessed(
               fileSystem, entryPointPackageJson, entryPointPackageJsonPath, propsToMarkAsProcessed);
+        } else {
+          logger.warn(
+              `Skipping ${entryPoint.name} : ${format} (no valid entry point file for this format).`);
+        }
       }
     }
 
@@ -264,3 +260,20 @@ function logInvalidEntryPoints(logger: Logger, invalidEntryPoints: InvalidEntryP
             invalidEntryPoint.missingDependencies.map(dep => ` - ${dep}`).join('\n'));
   });
 }
+
+function getFormatPathToPropertiesMap(packageJson: EntryPointPackageJson):
+    Map<string, EntryPointJsonProperty[]> {
+  const map = new Map<string, EntryPointJsonProperty[]>();
+
+  for (const prop of SUPPORTED_FORMAT_PROPERTIES) {
+    const formatPath = packageJson[prop];
+    if (formatPath) {
+      if (!map.has(formatPath)) {
+        map.set(formatPath, []);
+      }
+      map.get(formatPath) !.push(prop);
+    }
+  }
+
+  return map;
+}

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -163,6 +163,23 @@ runInEachFileSystem(() => {
              ]);
            });
       });
+
+      it('should skip all processing if the first matching `propertyToConsider` is marked as processed',
+         () => {
+           const logger = new MockLogger();
+           markPropertiesAsProcessed('@angular/common/http/testing', ['esm2015']);
+           mainNgcc({
+             basePath: '/node_modules',
+             targetEntryPointPath: '@angular/common/http/testing',
+             // Simulate a property that does not exist on the package.json and will be ignored.
+             propertiesToConsider: ['missing', 'esm2015', 'esm5'],
+             compileAllFormats: false, logger,
+           });
+
+           expect(logger.logs.debug).toContain([
+             'The target entry-point has already been processed'
+           ]);
+         });
     });
 
 
@@ -214,6 +231,33 @@ runInEachFileSystem(() => {
              typings: '0.0.0-PLACEHOLDER',
            });
          });
+
+      it('should mark all matching properties as processed in order not to compile them on a subsequent run',
+         () => {
+           const logger = new MockLogger();
+           const logs = logger.logs.debug;
+
+           // `fesm2015` and `es2015` map to the same file: `./fesm2015/common.js`
+           mainNgcc({
+             basePath: '/node_modules/@angular/common',
+             propertiesToConsider: ['fesm2015'], logger,
+           });
+
+           expect(logs).not.toContain(['Skipping @angular/common : es2015 (already compiled).']);
+           expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toEqual({
+             es2015: '0.0.0-PLACEHOLDER',
+             fesm2015: '0.0.0-PLACEHOLDER',
+             typings: '0.0.0-PLACEHOLDER',
+           });
+
+           // Now, compiling `es2015` should be a no-op.
+           mainNgcc({
+             basePath: '/node_modules/@angular/common',
+             propertiesToConsider: ['es2015'], logger,
+           });
+
+           expect(logs).toContain(['Skipping @angular/common : es2015 (already compiled).']);
+         });
     });
 
     describe('with compileAllFormats set to false', () => {
@@ -260,6 +304,7 @@ runInEachFileSystem(() => {
 
            });
            expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
+             fesm5: '0.0.0-PLACEHOLDER',
              module: '0.0.0-PLACEHOLDER',
              typings: '0.0.0-PLACEHOLDER',
            });
@@ -272,6 +317,7 @@ runInEachFileSystem(() => {
            });
            expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
              esm5: '0.0.0-PLACEHOLDER',
+             fesm5: '0.0.0-PLACEHOLDER',
              module: '0.0.0-PLACEHOLDER',
              typings: '0.0.0-PLACEHOLDER',
            });
@@ -346,6 +392,7 @@ runInEachFileSystem(() => {
         });
         expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
           es2015: '0.0.0-PLACEHOLDER',
+          fesm2015: '0.0.0-PLACEHOLDER',
           typings: '0.0.0-PLACEHOLDER',
         });
         expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
@@ -403,6 +450,7 @@ runInEachFileSystem(() => {
         });
         expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
           es2015: '0.0.0-PLACEHOLDER',
+          fesm2015: '0.0.0-PLACEHOLDER',
           typings: '0.0.0-PLACEHOLDER',
         });
       });
@@ -429,6 +477,7 @@ runInEachFileSystem(() => {
         // We process core but not core/testing.
         expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
           es2015: '0.0.0-PLACEHOLDER',
+          fesm2015: '0.0.0-PLACEHOLDER',
           typings: '0.0.0-PLACEHOLDER',
         });
         expect(loadPackage('@angular/core/testing').__processed_by_ivy_ngcc__).toBeUndefined();
@@ -436,6 +485,7 @@ runInEachFileSystem(() => {
         expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toBeUndefined();
         expect(loadPackage('@angular/common/http').__processed_by_ivy_ngcc__).toEqual({
           es2015: '0.0.0-PLACEHOLDER',
+          fesm2015: '0.0.0-PLACEHOLDER',
           typings: '0.0.0-PLACEHOLDER',
         });
       });