fix(ngcc): copy (and update) source-maps for unmodified source files (#…

…40429)

When using the `NewEntryPointWriter` we copy unmodified files over to the new
entry-point in addition to writing out the source files that are processed by ngcc.
But we were not copying over associated source-map files for these unmodified
source files, leading to warnings in downstream tooling.

Now we will also copy over source-maps that reside as siblings of unmodified
source files. We have to make sure that the sources of the source-map point
to the correct files, so we also update the `sourceRoot` property of the copied
source-map.

Fixes #40358

PR Close #40429

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

@@ -69,16 +69,49 @@ export class NewEntryPointFileWriter extends InPlaceFileWriter {
   protected copyBundle(
       bundle: EntryPointBundle, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath) {
     bundle.src.program.getSourceFiles().forEach(sourceFile => {
-      const relativePath = this.fs.relative(packagePath, absoluteFromSourceFile(sourceFile));
+      const originalPath = absoluteFromSourceFile(sourceFile);
+      const relativePath = this.fs.relative(packagePath, originalPath);
       const isInsidePackage = isLocalRelativePath(relativePath);
       if (!sourceFile.isDeclarationFile && isInsidePackage) {
-        const newFilePath = this.fs.resolve(ngccFolder, relativePath);
-        this.fs.ensureDir(this.fs.dirname(newFilePath));
-        this.fs.copyFile(absoluteFromSourceFile(sourceFile), newFilePath);
+        const newPath = this.fs.resolve(ngccFolder, relativePath);
+        this.fs.ensureDir(this.fs.dirname(newPath));
+        this.fs.copyFile(originalPath, newPath);
+        this.copyAndUpdateSourceMap(originalPath, newPath);
       }
     });
   }
 
+  /**
+   * If a source file has an associated source-map, then copy this, while updating its sourceRoot
+   * accordingly.
+   *
+   * For now don't try to parse the source for inline source-maps or external source-map links,
+   * since that is more complex and will slow ngcc down.
+   * Instead just check for a source-map file residing next to the source file, which is by far
+   * the most common case.
+   *
+   * @param originalSrcPath absolute path to the original source file being copied.
+   * @param newSrcPath absolute path to where the source will be written.
+   */
+  protected copyAndUpdateSourceMap(originalSrcPath: AbsoluteFsPath, newSrcPath: AbsoluteFsPath):
+      void {
+    const sourceMapPath = (originalSrcPath + '.map') as AbsoluteFsPath;
+    if (this.fs.exists(sourceMapPath)) {
+      try {
+        const sourceMap = JSON.parse(this.fs.readFile(sourceMapPath));
+        const newSourceMapPath = (newSrcPath + '.map') as AbsoluteFsPath;
+        const relativePath =
+            this.fs.relative(this.fs.dirname(newSourceMapPath), this.fs.dirname(sourceMapPath));
+        sourceMap['sourceRoot'] = this.fs.join(relativePath, sourceMap['sourceRoot'] || '.');
+        this.fs.ensureDir(this.fs.dirname(newSourceMapPath));
+        this.fs.writeFile(newSourceMapPath, JSON.stringify(sourceMap));
+      } catch (e) {
+        this.logger.warn(`Failed to process source-map at ${sourceMapPath}`);
+        this.logger.warn(e.message ?? e);
+      }
+    }
+  }
+
   protected writeFile(file: FileToWrite, packagePath: AbsoluteFsPath, ngccFolder: AbsoluteFsPath):
       void {
     if (isDtsPath(file.path.replace(/\.map$/, ''))) {

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

@@ -51,6 +51,12 @@ runInEachFileSystem(() => {
         {name: _('/node_modules/test/esm5.js'), contents: 'export function FooTop() {}'},
         {name: _('/node_modules/test/esm5.js.map'), contents: 'ORIGINAL MAPPING DATA'},
         {name: _('/node_modules/test/es2015/index.js'), contents: 'export {FooTop} from "./foo";'},
+        {
+          name: _('/node_modules/test/es2015/index.js.map'),
+          contents:
+              '{"version":3,"file":"index.js","sources":["../src/index.ts"],"mappings":"AAAA"}'
+        },
+        {name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'},
         {name: _('/node_modules/test/es2015/foo.js'), contents: 'export class FooTop {}'},
         {
           name: _('/node_modules/test/a/package.json'),
@@ -154,6 +160,85 @@ runInEachFileSystem(() => {
             .toEqual('export {FooTop} from "./foo";');
       });
 
+      it('should copy any source-map for unmodified files in the program (adding missing sourceRoot)',
+         () => {
+           // Ensure source-mapping for a non-processed source file `index.js`.
+           const sourceMap = {
+             version: 3,
+             file: 'index.js',
+             sources: ['../src/index.ts'],
+             mappings: 'AAAA',
+           };
+           loadTestFiles([
+             {
+               name: _('/node_modules/test/es2015/index.js.map'),
+               contents: JSON.stringify(sourceMap)
+             },
+             {name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
+           ]);
+
+           // Simulate that only the `foo.js` file was modified
+           const modifiedFiles = [{
+             path: _('/node_modules/test/es2015/foo.js'),
+             contents: 'export class FooTop {} // MODIFIED'
+           }];
+           fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
+
+           expect(JSON.parse(fs.readFile(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))))
+               .toEqual({...sourceMap, sourceRoot: '../../es2015'});
+         });
+
+      it('should copy any source-map for unmodified files in the program (updating sourceRoot)',
+         () => {
+           // Ensure source-mapping for a non-processed source file `index.js`.
+           const sourceMap = {
+             version: 3,
+             file: 'index.js',
+             sourceRoot: '../src',
+             sources: ['index.ts'],
+             mappings: 'AAAA',
+           };
+           loadTestFiles([
+             {
+               name: _('/node_modules/test/es2015/index.js.map'),
+               contents: JSON.stringify(sourceMap)
+             },
+             {name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
+           ]);
+
+           // Simulate that only the `foo.js` file was modified
+           const modifiedFiles = [{
+             path: _('/node_modules/test/es2015/foo.js'),
+             contents: 'export class FooTop {} // MODIFIED'
+           }];
+           fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
+
+           expect(JSON.parse(fs.readFile(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))))
+               .toEqual({...sourceMap, sourceRoot: '../../src'});
+         });
+
+      it('should ignore (with a warning) any invalid source-map for unmodified files in the program',
+         () => {
+           // Ensure source-mapping for a non-processed source file `index.js`.
+           loadTestFiles([
+             {name: _('/node_modules/test/es2015/index.js.map'), contents: 'INVALID JSON STRING'},
+             {name: _('/node_modules/test/src/index.ts'), contents: 'export {FooTop} from "./foo";'}
+           ]);
+
+           // Simulate that only the `foo.js` file was modified
+           const modifiedFiles = [{
+             path: _('/node_modules/test/es2015/foo.js'),
+             contents: 'export class FooTop {} // MODIFIED'
+           }];
+           fileWriter.writeBundle(esm2015bundle, modifiedFiles, ['es2015']);
+
+           expect(fs.exists(_('/node_modules/test/__ivy_ngcc__/es2015/index.js.map'))).toBe(false);
+           expect(logger.logs.warn).toEqual([
+             [`Failed to process source-map at ${_('/node_modules/test/es2015/index.js.map')}`],
+             ['Unexpected token I in JSON at position 0'],
+           ]);
+         });
+
       it('should update the package.json properties', () => {
         fileWriter.writeBundle(
             esm5bundle,