fix(ivy): ngcc - correctly update `package.json` when `createNewEntryPointFormats` is true (#32052)

Previously, when run with `createNewEntryPointFormats: true`, `ngcc`
would only update `package.json` with the new entry-point for the first
format property that mapped to a format-path. Subsequent properties
mapping to the same format-path would be detected as processed and not
have their new entry-point format recorded in `package.json`.

This commit fixes this by ensuring `package.json` is updated for all
matching format properties, when writing an `EntryPointBundle`.

PR Close #32052

packages/compiler-cli/ngcc/src/execution/api.ts

@@ -29,13 +29,6 @@ export type ExecuteFn = (analyzeFn: AnalyzeFn, createCompileFn: CreateCompileFn)
 
 /** Represents metadata related to the processing of an entry-point. */
 export interface EntryPointProcessingMetadata {
-  /**
-   * A mapping from a format property (i.e. an `EntryPointJsonProperty`) to the list of format
-   * properties that point to the same format-path and as a result need to be marked as processed,
-   * once the former is processed.
-   */
-  propertyToPropertiesToMarkAsProcessed: Map<EntryPointJsonProperty, EntryPointJsonProperty[]>;
-
   /**
    * Whether the typings for the entry-point have been successfully processed (or were already
    * processed).
@@ -60,6 +53,13 @@ export interface Task {
    */
   formatProperty: EntryPointJsonProperty;
 
+  /**
+   * The list of all format properties (including `task.formatProperty`) that should be marked as
+   * processed once the taksk has been completed, because they point to the format-path that will be
+   * processed as part of the task.
+   */
+  formatPropertiesToMarkAsProcessed: EntryPointJsonProperty[];
+
   /** Whether to also process typings for this entry-point as part of the task. */
   processDts: boolean;
 }

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

@@ -117,14 +117,15 @@ export function mainNgcc(
       let processDts = !hasProcessedTypings;
 
       for (const formatProperty of propertiesToProcess) {
-        tasks.push({entryPoint, formatProperty, processDts});
+        const formatPropertiesToMarkAsProcessed =
+            propertyToPropertiesToMarkAsProcessed.get(formatProperty) !;
+        tasks.push({entryPoint, formatProperty, formatPropertiesToMarkAsProcessed, processDts});
 
         // Only process typings for the first property (if not already processed).
         processDts = false;
       }
 
       processingMetadataPerEntryPoint.set(entryPoint.path, {
-        propertyToPropertiesToMarkAsProcessed,
         hasProcessedTypings,
         hasAnyProcessedFormat: false,
       });
@@ -139,7 +140,7 @@ export function mainNgcc(
     const transformer = new Transformer(fileSystem, logger);
 
     return (task: Task) => {
-      const {entryPoint, formatProperty, processDts} = task;
+      const {entryPoint, formatProperty, formatPropertiesToMarkAsProcessed, processDts} = task;
 
       const isCore = entryPoint.name === '@angular/core';  // Are we compiling the Angular core?
       const packageJson = entryPoint.packageJson;
@@ -171,7 +172,7 @@ export function mainNgcc(
       logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`);
 
       const transformedFiles = transformer.transform(bundle);
-      fileWriter.writeBundle(bundle, transformedFiles, formatProperty);
+      fileWriter.writeBundle(bundle, transformedFiles, formatPropertiesToMarkAsProcessed);
 
       onTaskCompleted(task, TaskProcessingOutcome.Processed);
     };
@@ -180,14 +181,15 @@ export function mainNgcc(
   // The function for actually planning and getting the work done.
   const executeFn: ExecuteFn = (analyzeFn: AnalyzeFn, createCompileFn: CreateCompileFn) => {
     const {processingMetadataPerEntryPoint, tasks} = analyzeFn();
-    const compile = createCompileFn(({entryPoint, formatProperty, processDts}, outcome) => {
+    const compile = createCompileFn((task, outcome) => {
+      const {entryPoint, formatPropertiesToMarkAsProcessed, processDts} = task;
       const processingMeta = processingMetadataPerEntryPoint.get(entryPoint.path) !;
       processingMeta.hasAnyProcessedFormat = true;
 
       if (outcome === TaskProcessingOutcome.Processed) {
         const packageJsonPath = fileSystem.resolve(entryPoint.path, 'package.json');
         const propsToMarkAsProcessed: (EntryPointJsonProperty | 'typings')[] =
-            processingMeta.propertyToPropertiesToMarkAsProcessed.get(formatProperty) !;
+            [...formatPropertiesToMarkAsProcessed];
 
         if (processDts) {
           processingMeta.hasProcessedTypings = true;

packages/compiler-cli/ngcc/src/writing/file_writer.ts

@@ -16,5 +16,5 @@ import {FileToWrite} from '../rendering/utils';
 export interface FileWriter {
   writeBundle(
       bundle: EntryPointBundle, transformedFiles: FileToWrite[],
-      formatProperty: EntryPointJsonProperty): void;
+      formatProperties: EntryPointJsonProperty[]): void;
 }

packages/compiler-cli/ngcc/src/writing/in_place_file_writer.ts

@@ -21,7 +21,7 @@ export class InPlaceFileWriter implements FileWriter {
 
   writeBundle(
       _bundle: EntryPointBundle, transformedFiles: FileToWrite[],
-      _formatProperty?: EntryPointJsonProperty) {
+      _formatProperties?: EntryPointJsonProperty[]) {
     transformedFiles.forEach(file => this.writeFileAndBackup(file));
   }
 

packages/compiler-cli/ngcc/src/writing/new_entry_point_file_writer.ts

@@ -27,13 +27,13 @@ const NGCC_DIRECTORY = '__ivy_ngcc__';
 export class NewEntryPointFileWriter extends InPlaceFileWriter {
   writeBundle(
       bundle: EntryPointBundle, transformedFiles: FileToWrite[],
-      formatProperty: EntryPointJsonProperty) {
+      formatProperties: EntryPointJsonProperty[]) {
     // The new folder is at the root of the overall package
     const entryPoint = bundle.entryPoint;
     const ngccFolder = join(entryPoint.package, NGCC_DIRECTORY);
     this.copyBundle(bundle, entryPoint.package, ngccFolder);
     transformedFiles.forEach(file => this.writeFile(file, entryPoint.package, ngccFolder));
-    this.updatePackageJson(entryPoint, formatProperty, ngccFolder);
+    this.updatePackageJson(entryPoint, formatProperties, ngccFolder);
   }
 
   protected copyBundle(
@@ -63,12 +63,18 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
   }
 
   protected updatePackageJson(
-      entryPoint: EntryPoint, formatProperty: EntryPointJsonProperty, ngccFolder: AbsoluteFsPath) {
+      entryPoint: EntryPoint, formatProperties: EntryPointJsonProperty[],
-    const formatPath = join(entryPoint.path, entryPoint.packageJson[formatProperty] !);
+      ngccFolder: AbsoluteFsPath) {
-    const newFormatPath = join(ngccFolder, relative(entryPoint.package, formatPath));
+    const packageJson = entryPoint.packageJson;
-    const newFormatProperty = formatProperty + '_ivy_ngcc';
+
-    (entryPoint.packageJson as any)[newFormatProperty] = relative(entryPoint.path, newFormatPath);
+    for (const formatProperty of formatProperties) {
+      const formatPath = join(entryPoint.path, packageJson[formatProperty] !);
+      const newFormatPath = join(ngccFolder, relative(entryPoint.package, formatPath));
+      const newFormatProperty = formatProperty + '_ivy_ngcc';
+      (packageJson as any)[newFormatProperty] = relative(entryPoint.path, newFormatPath);
+    }
+
     this.fs.writeFile(
-        join(entryPoint.path, 'package.json'), JSON.stringify(entryPoint.packageJson));
+        join(entryPoint.path, 'package.json'), `${JSON.stringify(packageJson, null, 2)}\n`);
   }
 }

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

@@ -382,6 +382,28 @@ runInEachFileSystem(() => {
             .toMatch(ANGULAR_CORE_IMPORT_REGEX);
         expect(fs.exists(_(`/node_modules/@angular/common/common.d.ts.__ivy_ngcc_bak`))).toBe(true);
       });
+
+      it('should update `package.json` for all matching format properties', () => {
+        mainNgcc({
+          basePath: '/node_modules/@angular/core',
+          createNewEntryPointFormats: true,
+          propertiesToConsider: ['fesm2015', 'fesm5'],
+        });
+
+        const pkg: any = loadPackage('@angular/core');
+
+        // `es2015` is an alias of `fesm2015`.
+        expect(pkg.fesm2015).toEqual('./fesm2015/core.js');
+        expect(pkg.es2015).toEqual('./fesm2015/core.js');
+        expect(pkg.fesm2015_ivy_ngcc).toEqual('__ivy_ngcc__/fesm2015/core.js');
+        expect(pkg.es2015_ivy_ngcc).toEqual('__ivy_ngcc__/fesm2015/core.js');
+
+        // `module` is an alias of `fesm5`.
+        expect(pkg.fesm5).toEqual('./fesm5/core.js');
+        expect(pkg.module).toEqual('./fesm5/core.js');
+        expect(pkg.fesm5_ivy_ngcc).toEqual('__ivy_ngcc__/fesm5/core.js');
+        expect(pkg.module_ivy_ngcc).toEqual('__ivy_ngcc__/fesm5/core.js');
+      });
     });
 
     describe('logger', () => {

packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts

@@ -32,8 +32,15 @@ runInEachFileSystem(() => {
 
         {
           name: _('/node_modules/test/package.json'),
-          contents:
+          contents: `
-              '{"module": "./esm5.js", "es2015": "./es2015/index.js", "typings": "./index.d.ts"}'
+            {
+              "module": "./esm5.js",
+              "fesm2015": "./es2015/index.js",
+              "fesm5": "./esm5.js",
+              "es2015": "./es2015/index.js",
+              "typings": "./index.d.ts"
+            }
+          `,
         },
         {name: _('/node_modules/test/index.d.ts'), contents: 'export declare class FooTop {}'},
         {name: _('/node_modules/test/index.d.ts.map'), contents: 'ORIGINAL MAPPING DATA'},
@@ -44,8 +51,15 @@ runInEachFileSystem(() => {
         {name: _('/node_modules/test/es2015/foo.js'), contents: 'export class FooTop {}'},
         {
           name: _('/node_modules/test/a/package.json'),
-          contents:
+          contents: `
-              `{"module": "./esm5.js", "es2015": "./es2015/index.js", "typings": "./index.d.ts"}`
+            {
+              "module": "./esm5.js",
+              "fesm2015": "./es2015/index.js",
+              "fesm5": "./esm5.js",
+              "es2015": "./es2015/index.js",
+              "typings": "./index.d.ts"
+            }
+          `,
         },
         {name: _('/node_modules/test/a/index.d.ts'), contents: 'export declare class FooA {}'},
         {name: _('/node_modules/test/a/index.metadata.json'), contents: '...'},
@@ -104,7 +118,7 @@ runInEachFileSystem(() => {
               },
               {path: _('/node_modules/test/esm5.js.map'), contents: 'MODIFIED MAPPING DATA'},
             ],
-            'module');
+            ['module']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/esm5.js')))
             .toEqual('export function FooTop() {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/esm5.js'))).toEqual('export function FooTop() {}');
@@ -122,7 +136,7 @@ runInEachFileSystem(() => {
                 contents: 'export class FooTop {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/es2015/foo.js')))
             .toEqual('export class FooTop {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/es2015/foo.js')))
@@ -142,7 +156,7 @@ runInEachFileSystem(() => {
                 contents: 'export function FooTop() {} // MODIFIED'
               },
             ],
-            'module');
+            ['module']);
         expect(loadPackageJson(fs, '/node_modules/test')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '__ivy_ngcc__/esm5.js',
         }));
@@ -155,13 +169,45 @@ runInEachFileSystem(() => {
                 contents: 'export class FooTop {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(loadPackageJson(fs, '/node_modules/test')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '__ivy_ngcc__/esm5.js',
           es2015_ivy_ngcc: '__ivy_ngcc__/es2015/index.js',
         }));
       });
 
+      it('should be able to update multiple package.json properties at once', () => {
+        fileWriter.writeBundle(
+            esm5bundle,
+            [
+              {
+                path: _('/node_modules/test/esm5.js'),
+                contents: 'export function FooTop() {} // MODIFIED'
+              },
+            ],
+            ['module', 'fesm5']);
+        expect(loadPackageJson(fs, '/node_modules/test')).toEqual(jasmine.objectContaining({
+          module_ivy_ngcc: '__ivy_ngcc__/esm5.js',
+          fesm5_ivy_ngcc: '__ivy_ngcc__/esm5.js',
+        }));
+
+        fileWriter.writeBundle(
+            esm2015bundle,
+            [
+              {
+                path: _('/node_modules/test/es2015/foo.js'),
+                contents: 'export class FooTop {} // MODIFIED'
+              },
+            ],
+            ['es2015', 'fesm2015']);
+        expect(loadPackageJson(fs, '/node_modules/test')).toEqual(jasmine.objectContaining({
+          module_ivy_ngcc: '__ivy_ngcc__/esm5.js',
+          fesm5_ivy_ngcc: '__ivy_ngcc__/esm5.js',
+          es2015_ivy_ngcc: '__ivy_ngcc__/es2015/index.js',
+          fesm2015_ivy_ngcc: '__ivy_ngcc__/es2015/index.js',
+        }));
+      });
+
       it('should overwrite and backup typings files', () => {
         fileWriter.writeBundle(
             esm2015bundle,
@@ -172,7 +218,7 @@ runInEachFileSystem(() => {
               },
               {path: _('/node_modules/test/index.d.ts.map'), contents: 'MODIFIED MAPPING DATA'},
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/index.d.ts')))
             .toEqual('export declare class FooTop {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/index.d.ts.__ivy_ngcc_bak')))
@@ -207,7 +253,7 @@ runInEachFileSystem(() => {
                 contents: 'export function FooA() {} // MODIFIED'
               },
             ],
-            'module');
+            ['module']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/a/esm5.js')))
             .toEqual('export function FooA() {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/a/esm5.js'))).toEqual('export function FooA() {}');
@@ -222,7 +268,7 @@ runInEachFileSystem(() => {
                 contents: 'export class FooA {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/a/es2015/foo.js')))
             .toEqual('export class FooA {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/a/es2015/foo.js')))
@@ -242,7 +288,7 @@ runInEachFileSystem(() => {
                 contents: 'export function FooA() {} // MODIFIED'
               },
             ],
-            'module');
+            ['module']);
         expect(loadPackageJson(fs, '/node_modules/test/a')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
         }));
@@ -255,13 +301,45 @@ runInEachFileSystem(() => {
                 contents: 'export class FooA {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(loadPackageJson(fs, '/node_modules/test/a')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
           es2015_ivy_ngcc: '../__ivy_ngcc__/a/es2015/index.js',
         }));
       });
 
+      it('should be able to update multiple package.json properties at once', () => {
+        fileWriter.writeBundle(
+            esm5bundle,
+            [
+              {
+                path: _('/node_modules/test/a/esm5.js'),
+                contents: 'export function FooA() {} // MODIFIED'
+              },
+            ],
+            ['module', 'fesm5']);
+        expect(loadPackageJson(fs, '/node_modules/test/a')).toEqual(jasmine.objectContaining({
+          module_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
+          fesm5_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
+        }));
+
+        fileWriter.writeBundle(
+            esm2015bundle,
+            [
+              {
+                path: _('/node_modules/test/a/es2015/foo.js'),
+                contents: 'export class FooA {} // MODIFIED'
+              },
+            ],
+            ['es2015', 'fesm2015']);
+        expect(loadPackageJson(fs, '/node_modules/test/a')).toEqual(jasmine.objectContaining({
+          module_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
+          fesm5_ivy_ngcc: '../__ivy_ngcc__/a/esm5.js',
+          es2015_ivy_ngcc: '../__ivy_ngcc__/a/es2015/index.js',
+          fesm2015_ivy_ngcc: '../__ivy_ngcc__/a/es2015/index.js',
+        }));
+      });
+
       it('should overwrite and backup typings files', () => {
         fileWriter.writeBundle(
             esm2015bundle,
@@ -271,7 +349,7 @@ runInEachFileSystem(() => {
                 contents: 'export declare class FooA {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/a/index.d.ts')))
             .toEqual('export declare class FooA {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/a/index.d.ts.__ivy_ngcc_bak')))
@@ -300,7 +378,7 @@ runInEachFileSystem(() => {
                 contents: 'export function FooB() {} // MODIFIED'
               },
             ],
-            'module');
+            ['module']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/lib/esm5.js')))
             .toEqual('export function FooB() {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/lib/esm5.js')))
@@ -316,7 +394,7 @@ runInEachFileSystem(() => {
                 contents: 'export class FooB {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/__ivy_ngcc__/lib/es2015/foo.js')))
             .toEqual('export class FooB {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/lib/es2015/foo.js')))
@@ -337,7 +415,7 @@ runInEachFileSystem(() => {
                    contents: 'export class FooB {} // MODIFIED'
                  },
                ],
-               'es2015');
+               ['es2015']);
            expect(fs.exists(_('/node_modules/test/__ivy_ngcc__/a/index.d.ts'))).toEqual(false);
          });
 
@@ -350,7 +428,7 @@ runInEachFileSystem(() => {
                 contents: 'export class FooB {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.exists(_('/node_modules/test/other/index.d.ts'))).toEqual(false);
         expect(fs.exists(_('/node_modules/test/events/events.js'))).toEqual(false);
       });
@@ -364,7 +442,7 @@ runInEachFileSystem(() => {
                 contents: 'export function FooB() {} // MODIFIED'
               },
             ],
-            'module');
+            ['module']);
         expect(loadPackageJson(fs, '/node_modules/test/b')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '../__ivy_ngcc__/lib/esm5.js',
         }));
@@ -377,7 +455,7 @@ runInEachFileSystem(() => {
                 contents: 'export class FooB {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(loadPackageJson(fs, '/node_modules/test/b')).toEqual(jasmine.objectContaining({
           module_ivy_ngcc: '../__ivy_ngcc__/lib/esm5.js',
           es2015_ivy_ngcc: '../__ivy_ngcc__/lib/es2015/index.js',
@@ -393,7 +471,7 @@ runInEachFileSystem(() => {
                 contents: 'export declare class FooB {} // MODIFIED'
               },
             ],
-            'es2015');
+            ['es2015']);
         expect(fs.readFile(_('/node_modules/test/typings/index.d.ts')))
             .toEqual('export declare class FooB {} // MODIFIED');
         expect(fs.readFile(_('/node_modules/test/typings/index.d.ts.__ivy_ngcc_bak')))