feat(compiler-cli): generate global imports in local compilation mode (#53543)

pmvald · thePunderWoman · commit 3263df23f2f4 · 2024-01-30T15:05:43.000Z
With option `generateExtraImportsInLocalMode` set in local compilation mode, the compiler generates extra side effect imports using this rule: any external module from which an identifier is imported into an NgModule will be added as side effect import to every file in the compilation unit. To illustrate this better assume the compilation unit has source files `a.ts` and `b.ts`, and: ``` // a.ts import {SomeExternalStuff} from 'path/to/some_where'; import {SomeExternalStuff2} from 'path/to/some_where2'; ... @NgModule({imports: [SomeExternalStuff]}) ``` then the extra import `import "path/to/some_where"` will be added to both `a.js` and `b.js`. Note that this is not the case for `import "path/to/some_where2"` though, since the symbol `SomeExternalStuff2` is not imported into any NgModule. The math behind this mechanism is, in local compilation mode we cannot resolve component external dependencies fully. For example if a component in `a.ts` uses an external component defined in an external file `some_external_comp.ts` then we can generate the import to this file in `a.js`. Instead, we want to generate an import to a file that "gurantees" that `a.js` is placed after `some_external_comp.js` in the bundle. Now since the component in `some_external_comp.ts` is used in `a.ts`, then there must be a chain of imports starting from the NgModule that declares the component in `a.ts` to the component in `some_external_comp.ts`. This chain means some file in the same compilation unit as `a.ts` should import some external NgModule which includes `some_external_comp.ts` in its transitive closure and import it to some NgModule. So by adding this import to `a.js` we ensure that the bundling will have the right order. PR Close #53543
diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts
@@ -10,10 +10,10 @@ import {compileClassMetadata, compileDeclareClassMetadata, compileDeclareInjecto
 import ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics';
-import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../../imports';
+import {assertSuccessfulReferenceEmit, LocalCompilationExtraImportsTracker, Reference, ReferenceEmitter} from '../../../imports';
 import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol,} from '../../../incremental/semantic_graph';
 import {ExportedProviderStatusResolver, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata';
-import {PartialEvaluator, ResolvedValue, SyntheticValue} from '../../../partial_evaluator';
+import {DynamicValue, PartialEvaluator, ResolvedValue, SyntheticValue} from '../../../partial_evaluator';
 import {PerfEvent, PerfRecorder} from '../../../perf';
 import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral,} from '../../../reflection';
 import {LocalModuleScopeRegistry, ScopeData} from '../../../scope';
@@ -179,7 +179,9 @@ export class NgModuleDecoratorHandler implements
       private onlyPublishPublicTypings: boolean,
       private injectableRegistry: InjectableClassRegistry, private perf: PerfRecorder,
       private includeClassMetadata: boolean, private includeSelectorScope: boolean,
-      private readonly compilationMode: CompilationMode) {}
+      private readonly compilationMode: CompilationMode,
+      private readonly localCompilationExtraImportsTracker: LocalCompilationExtraImportsTracker|
+      null) {}
 
   readonly precedence = HandlerPrecedence.PRIMARY;
   readonly name = 'NgModuleDecoratorHandler';
@@ -240,9 +242,10 @@ export class NgModuleDecoratorHandler implements
     const rawDeclarations: ts.Expression|null = ngModule.get('declarations') ?? null;
     if (this.compilationMode !== CompilationMode.LOCAL && rawDeclarations !== null) {
       const declarationMeta = this.evaluator.evaluate(rawDeclarations, forwardRefResolver);
-      declarationRefs =
-          this.resolveTypeList(rawDeclarations, declarationMeta, name, 'declarations', 0)
-              .references;
+      declarationRefs = this.resolveTypeList(
+                                rawDeclarations, declarationMeta, name, 'declarations', 0,
+                                /* allowUnresolvedReferences */ false)
+                            .references;
 
       // Look through the declarations to make sure they're all a part of the current compilation.
       for (const ref of declarationRefs) {
@@ -267,17 +270,41 @@ export class NgModuleDecoratorHandler implements
     // Resolving imports
     let importRefs: Reference<ClassDeclaration>[] = [];
     let rawImports: ts.Expression|null = ngModule.get('imports') ?? null;
-    if (this.compilationMode !== CompilationMode.LOCAL && rawImports !== null) {
+    if (rawImports !== null) {
       const importsMeta = this.evaluator.evaluate(rawImports, moduleResolvers);
-      importRefs = this.resolveTypeList(rawImports, importsMeta, name, 'imports', 0).references;
+
+      const result = this.resolveTypeList(
+          rawImports, importsMeta, name, 'imports', 0,
+          this.compilationMode === CompilationMode.LOCAL);
+
+      if (this.compilationMode === CompilationMode.LOCAL &&
+          this.localCompilationExtraImportsTracker !== null) {
+        // For generating extra imports in local mode, the NgModule imports that are from external
+        // files (i.e., outside of the compilation unit) are to be added to all the files in the
+        // compilation unit. This is because any external component that is a dependency of some
+        // component in the compilation unit must be imported by one of these NgModule's external
+        // imports (or the external component cannot be a dependency of that internal component).
+        // This approach can be further optimized by adding these NgModule external imports to a
+        // subset of files in the compilation unit and not all. See comments in {@link
+        // LocalCompilationExtraImportsTracker} and {@link
+        // LocalCompilationExtraImportsTracker#addGlobalImportFromIdentifier} for more details.
+        for (const d of result.dynamicValues) {
+          this.localCompilationExtraImportsTracker.addGlobalImportFromIdentifier(d.node);
+        }
+      }
+
+      importRefs = result.references;
     }
 
     // Resolving exports
     let exportRefs: Reference<ClassDeclaration>[] = [];
     const rawExports: ts.Expression|null = ngModule.get('exports') ?? null;
     if (this.compilationMode !== CompilationMode.LOCAL && rawExports !== null) {
       const exportsMeta = this.evaluator.evaluate(rawExports, moduleResolvers);
-      exportRefs = this.resolveTypeList(rawExports, exportsMeta, name, 'exports', 0).references;
+      exportRefs = this.resolveTypeList(
+                           rawExports, exportsMeta, name, 'exports', 0,
+                           /* allowUnresolvedReferences */ false)
+                       .references;
       this.referencesRegistry.add(node, ...exportRefs);
     }
 
@@ -286,8 +313,10 @@ export class NgModuleDecoratorHandler implements
     const rawBootstrap: ts.Expression|null = ngModule.get('bootstrap') ?? null;
     if (this.compilationMode !== CompilationMode.LOCAL && rawBootstrap !== null) {
       const bootstrapMeta = this.evaluator.evaluate(rawBootstrap, forwardRefResolver);
-      bootstrapRefs =
-          this.resolveTypeList(rawBootstrap, bootstrapMeta, name, 'bootstrap', 0).references;
+      bootstrapRefs = this.resolveTypeList(
+                              rawBootstrap, bootstrapMeta, name, 'bootstrap', 0,
+                              /* allowUnresolvedReferences */ false)
+                          .references;
 
       // Verify that the `@NgModule.bootstrap` list doesn't have Standalone Components.
       for (const ref of bootstrapRefs) {
@@ -425,8 +454,9 @@ export class NgModuleDecoratorHandler implements
       for (const importExpr of topLevelExpressions) {
         const resolved = this.evaluator.evaluate(importExpr, moduleResolvers);
 
-        const {references, hasModuleWithProviders} =
-            this.resolveTypeList(importExpr, [resolved], node.name.text, 'imports', absoluteIndex);
+        const {references, hasModuleWithProviders} = this.resolveTypeList(
+            importExpr, [resolved], node.name.text, 'imports', absoluteIndex,
+            /* allowUnresolvedReferences */ false);
         absoluteIndex += references.length;
 
         topLevelImports.push({
@@ -840,11 +870,24 @@ export class NgModuleDecoratorHandler implements
    */
   private resolveTypeList(
       expr: ts.Node, resolvedList: ResolvedValue, className: string, arrayName: string,
-      absoluteIndex: number):
-      {references: Reference<ClassDeclaration>[], hasModuleWithProviders: boolean} {
+      absoluteIndex: number, allowUnresolvedReferences: boolean): {
+    references: Reference<ClassDeclaration>[],
+    hasModuleWithProviders: boolean,
+    dynamicValues: DynamicValue[]
+  } {
     let hasModuleWithProviders = false;
     const refList: Reference<ClassDeclaration>[] = [];
+    const dynamicValueSet = new Set<DynamicValue>();
+
     if (!Array.isArray(resolvedList)) {
+      if (allowUnresolvedReferences) {
+        return {
+          references: [],
+          hasModuleWithProviders: false,
+          dynamicValues: [],
+        };
+      }
+
       throw createValueHasWrongTypeError(
           expr, resolvedList,
           `Expected array when reading the NgModule.${arrayName} of ${className}`);
@@ -864,9 +907,14 @@ export class NgModuleDecoratorHandler implements
 
       if (Array.isArray(entry)) {
         // Recurse into nested arrays.
-        const recursiveResult =
-            this.resolveTypeList(expr, entry, className, arrayName, absoluteIndex);
+        const recursiveResult = this.resolveTypeList(
+            expr, entry, className, arrayName, absoluteIndex, allowUnresolvedReferences);
         refList.push(...recursiveResult.references);
+
+        for (const d of recursiveResult.dynamicValues) {
+          dynamicValueSet.add(d);
+        }
+
         absoluteIndex += recursiveResult.references.length;
         hasModuleWithProviders = hasModuleWithProviders || recursiveResult.hasModuleWithProviders;
       } else if (entry instanceof Reference) {
@@ -878,6 +926,9 @@ export class NgModuleDecoratorHandler implements
         }
         refList.push(entry);
         absoluteIndex += 1;
+      } else if (entry instanceof DynamicValue && allowUnresolvedReferences) {
+        dynamicValueSet.add(entry);
+        continue;
       } else {
         // TODO(alxhub): Produce a better diagnostic here - the array index may be an inner array.
         throw createValueHasWrongTypeError(
@@ -890,6 +941,7 @@ export class NgModuleDecoratorHandler implements
     return {
       references: refList,
       hasModuleWithProviders,
+      dynamicValues: [...dynamicValueSet],
     };
   }
 }
diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts
@@ -44,7 +44,7 @@ function setup(program: ts.Program, compilationMode = CompilationMode.FULL) {
       /* isCore */ false, refEmitter,
       /* annotateForClosureCompiler */ false,
       /* onlyPublishPublicTypings */ false, injectableRegistry, NOOP_PERF_RECORDER, true, true,
-      compilationMode);
+      compilationMode, /* localCompilationExtraImportsTracker */ null);
 
   return {handler, reflectionHost};
 }
diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts
@@ -1097,7 +1097,7 @@ export class NgCompiler {
 
     let localCompilationExtraImportsTracker: LocalCompilationExtraImportsTracker|null = null;
     if (compilationMode === CompilationMode.LOCAL && this.options.generateExtraImportsInLocalMode) {
-      localCompilationExtraImportsTracker = new LocalCompilationExtraImportsTracker();
+      localCompilationExtraImportsTracker = new LocalCompilationExtraImportsTracker(checker);
     }
 
     // Cycles are handled in full compilation mode by "remote scoping".
@@ -1172,7 +1172,7 @@ export class NgCompiler {
           exportedProviderStatusResolver, semanticDepGraphUpdater, isCore, refEmitter,
           this.closureCompilerEnabled, this.options.onlyPublishPublicTypingsForNgModules ?? false,
           injectableRegistry, this.delegatingPerfRecorder, supportTestBed, supportJitMode,
-          compilationMode),
+          compilationMode, localCompilationExtraImportsTracker),
     ];
 
     const traitCompiler = new TraitCompiler(
diff --git a/packages/compiler-cli/src/ngtsc/imports/src/local_compilation_extra_imports_tracker.ts b/packages/compiler-cli/src/ngtsc/imports/src/local_compilation_extra_imports_tracker.ts
@@ -8,6 +8,8 @@
 
 import ts from 'typescript';
 
+import {getContainingImportDeclaration} from '../../reflection/src/typescript';
+
 
 /**
  * A tool to track extra imports to be added to the generated files in the local compilation mode.
@@ -32,6 +34,10 @@ import ts from 'typescript';
  *
  */
 export class LocalCompilationExtraImportsTracker {
+  private readonly globalImportsSet = new Set<string>();
+
+  constructor(private readonly typeChecker: ts.TypeChecker) {}
+
   /**
    * Adds an extra import to be added to the generated file of a specific source file.
    */
@@ -51,14 +57,41 @@ export class LocalCompilationExtraImportsTracker {
    * to smallest possible candidate files instead of all files.
    */
   addGlobalImportFromIdentifier(node: ts.Node): void {
-    // TODO(pmvald): Implement this method.
+    let identifier: ts.Identifier|null = null;
+    if (ts.isIdentifier(node)) {
+      identifier = node;
+    } else if (ts.isPropertyAccessExpression(node) && ts.isIdentifier(node.expression)) {
+      identifier = node.expression;
+    }
+
+    if (identifier === null) {
+      return;
+    }
+
+    const sym = this.typeChecker.getSymbolAtLocation(identifier);
+    if (!sym?.declarations?.length) {
+      return;
+    }
+
+
+    const importClause = sym.declarations[0];
+    const decl = getContainingImportDeclaration(importClause);
+
+    if (decl !== null) {
+      this.globalImportsSet.add(removeQuotations(decl.moduleSpecifier.getText()));
+    }
   }
 
   /**
    * Returns the list of all module names that the given file should include as its extra imports.
    */
   getImportsForFile(sf: ts.SourceFile): string[] {
-    // TODO(pmvald): Implement this method.
-    return [];
+    return [
+      ...this.globalImportsSet,
+    ];
   }
 }
+
+function removeQuotations(s: string): string {
+  return s.substring(1, s.length - 1).trim();
+}
diff --git a/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts b/packages/compiler-cli/test/ngtsc/local_compilation_spec.ts

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ function setup(program: ts.Program, compilationMode = CompilationMode.FULL) {`
`44`	`44`	`/* isCore */ false, refEmitter,`
`45`	`45`	`/* annotateForClosureCompiler */ false,`
`46`	`46`	`/* onlyPublishPublicTypings */ false, injectableRegistry, NOOP_PERF_RECORDER, true, true,`
`47`		`- compilationMode);`
	`47`	`+ compilationMode, /* localCompilationExtraImportsTracker */ null);`
`48`	`48`
`49`	`49`	`return {handler, reflectionHost};`
`50`	`50`	`}`