fix(compiler-cli): do not duplicate repeated source-files in rendered source-maps (#40237)

When a source-map/source-file tree has nodes that refer to the same file, the
flattened source-map rendering was those files multiple times, rather than
consolidating them into a single source-map source.

PR Close #40237

Author: petebacondarwin

packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file.ts

@@ -50,21 +50,27 @@ export class SourceFile {
    * Render the raw source map generated from the flattened mappings.
    */
   renderFlattenedSourceMap(): RawSourceMap {
-    const sources: SourceFile[] = [];
-    const names: string[] = [];
-
+    const sources = new IndexedMap<string, string>();
+    const names = new IndexedSet<string>();
     const mappings: SourceMapMappings = [];
+    const sourcePathDir = this.fs.dirname(this.sourcePath);
+    // Computing the relative path can be expensive, and we are likely to have the same path for
+    // many (if not all!) mappings.
+    const relativeSourcePathCache =
+        new Cache<string, string>(input => this.fs.relative(sourcePathDir, input));
 
     for (const mapping of this.flattenedMappings) {
-      const sourceIndex = findIndexOrAdd(sources, mapping.originalSource);
+      const sourceIndex = sources.set(
+          relativeSourcePathCache.get(mapping.originalSource.sourcePath),
+          mapping.originalSource.contents);
       const mappingArray: SourceMapSegment = [
         mapping.generatedSegment.column,
         sourceIndex,
         mapping.originalSegment.line,
         mapping.originalSegment.column,
       ];
       if (mapping.name !== undefined) {
-        const nameIndex = findIndexOrAdd(names, mapping.name);
+        const nameIndex = names.add(mapping.name);
         mappingArray.push(nameIndex);
       }
 
@@ -77,14 +83,13 @@ export class SourceFile {
       mappings[line].push(mappingArray);
     }
 
-    const sourcePathDir = this.fs.dirname(this.sourcePath);
     const sourceMap: RawSourceMap = {
       version: 3,
       file: this.fs.relative(sourcePathDir, this.sourcePath),
-      sources: sources.map(sf => this.fs.relative(sourcePathDir, sf.sourcePath)),
-      names,
+      sources: sources.keys,
+      names: names.values,
       mappings: encode(mappings),
-      sourcesContent: sources.map(sf => sf.contents),
+      sourcesContent: sources.values,
     };
     return sourceMap;
   }
@@ -259,23 +264,6 @@ export interface Mapping {
   readonly name?: string;
 }
 
-/**
- * Find the index of `item` in the `items` array.
- * If it is not found, then push `item` to the end of the array and return its new index.
- *
- * @param items the collection in which to look for `item`.
- * @param item the item to look for.
- * @returns the index of the `item` in the `items` array.
- */
-function findIndexOrAdd<T>(items: T[], item: T): number {
-  const itemIndex = items.indexOf(item);
-  if (itemIndex > -1) {
-    return itemIndex;
-  } else {
-    items.push(item);
-    return items.length - 1;
-  }
-}
 
 
 /**
@@ -448,3 +436,96 @@ export function computeStartOfLinePositions(str: string) {
 function computeLineLengths(str: string): number[] {
   return (str.split(/\n/)).map(s => s.length);
 }
+
+/**
+ * A collection of mappings between `keys` and `values` stored in the order in which the keys are
+ * first seen.
+ *
+ * The difference between this and a standard `Map` is that when you add a key-value pair the index
+ * of the `key` is returned.
+ */
+class IndexedMap<K, V> {
+  private map = new Map<K, number>();
+
+  /**
+   * An array of keys added to this map.
+   *
+   * This array is guaranteed to be in the order of the first time the key was added to the map.
+   */
+  readonly keys: K[] = [];
+
+  /**
+   * An array of values added to this map.
+   *
+   * This array is guaranteed to be in the order of the first time the associated key was added to
+   * the map.
+   */
+  readonly values: V[] = [];
+
+  /**
+   * Associate the `value` with the `key` and return the index of the key in the collection.
+   *
+   * If the `key` already exists then the `value` is not set and the index of that `key` is
+   * returned; otherwise the `key` and `value` are stored and the index of the new `key` is
+   * returned.
+   *
+   * @param key the key to associated with the `value`.
+   * @param value the value to associated with the `key`.
+   * @returns the index of the `key` in the `keys` array.
+   */
+  set(key: K, value: V): number {
+    if (this.map.has(key)) {
+      return this.map.get(key)!;
+    }
+    const index = this.values.push(value) - 1;
+    this.keys.push(key);
+    this.map.set(key, index);
+    return index;
+  }
+}
+
+/**
+ * A collection of `values` stored in the order in which they were added.
+ *
+ * The difference between this and a standard `Set` is that when you add a value the index of that
+ * item is returned.
+ */
+class IndexedSet<V> {
+  private map = new Map<V, number>();
+
+  /**
+   * An array of values added to this set.
+   * This array is guaranteed to be in the order of the first time the value was added to the set.
+   */
+  readonly values: V[] = [];
+
+  /**
+   * Add the `value` to the `values` array, if it doesn't already exist; returning the index of the
+   * `value` in the `values` array.
+   *
+   * If the `value` already exists then the index of that `value` is returned, otherwise the new
+   * `value` is stored and the new index returned.
+   *
+   * @param value the value to add to the set.
+   * @returns the index of the `value` in the `values` array.
+   */
+  add(value: V): number {
+    if (this.map.has(value)) {
+      return this.map.get(value)!;
+    }
+    const index = this.values.push(value) - 1;
+    this.map.set(value, index);
+    return index;
+  }
+}
+
+class Cache<Input, Cached> {
+  private map = new Map<Input, Cached>();
+  constructor(private computeFn: (input: Input) => Cached) {}
+  get(input: Input): Cached {
+    if (!this.map.has(input)) {
+      this.map.set(input, this.computeFn(input));
+    }
+    return this.map.get(input)!;
+  }
+}

packages/compiler-cli/src/ngtsc/sourcemaps/test/source_file_spec.ts

@@ -522,6 +522,42 @@ runInEachFileSystem(() => {
           expect(aTocSourceMap.sourcesContent).toEqual(['abcdef']);
           expect(aTocSourceMap.mappings).toEqual(aToBSourceMap.mappings);
         });
+
+        it('should consolidate source-files with the same relative path', () => {
+          const cSource1 = new SourceFile(_('/foo/src/lib/c.js'), 'bcd123e', null, false, [], fs);
+          const cSource2 = new SourceFile(_('/foo/src/lib/c.js'), 'bcd123e', null, false, [], fs);
+
+          const bToCSourceMap: RawSourceMap = {
+            mappings: encode([[[1, 0, 0, 0], [4, 0, 0, 3], [4, 0, 0, 6], [5, 0, 0, 7]]]),
+            names: [],
+            sources: ['c.js'],
+            version: 3
+          };
+          const bSource = new SourceFile(
+              _('/foo/src/lib/b.js'), 'abcdef', bToCSourceMap, false, [cSource1], fs);
+
+          const aToBCSourceMap: RawSourceMap = {
+            mappings:
+                encode([[[0, 0, 0, 0], [2, 0, 0, 3], [4, 0, 0, 2], [5, 0, 0, 5], [6, 1, 0, 3]]]),
+            names: [],
+            sources: ['lib/b.js', 'lib/c.js'],
+            version: 3
+          };
+          const aSource = new SourceFile(
+              _('/foo/src/a.js'), 'abdecf123', aToBCSourceMap, false, [bSource, cSource2], fs);
+
+          const aTocSourceMap = aSource.renderFlattenedSourceMap();
+          expect(aTocSourceMap.version).toEqual(3);
+          expect(aTocSourceMap.file).toEqual('a.js');
+          expect(aTocSourceMap.names).toEqual([]);
+          expect(aTocSourceMap.sourceRoot).toBeUndefined();
+          expect(aTocSourceMap.sources).toEqual(['lib/c.js']);
+          expect(aTocSourceMap.sourcesContent).toEqual(['bcd123e']);
+          expect(aTocSourceMap.mappings).toEqual(encode([[
+            [1, 0, 0, 0], [2, 0, 0, 2], [3, 0, 0, 3], [3, 0, 0, 6], [4, 0, 0, 1], [5, 0, 0, 7],
+            [6, 0, 0, 3]
+          ]]));
+        });
       });
 
       describe('getOriginalLocation()', () => {