refactor(compiler-cli): introduce `onlyPublishPublicTypingsForNgModul…

…es` (#45894)

When generating .d.ts metadata for NgModules, by default we emit type
references to their declarations, imports, and exports. However, this
information is not necessarily useful to consumers. References to private
directives (those that aren't exported by the NgModule) for example aren't
at all useful as they can only affect other components declared in the
NgModule. References to imports are of limited usefulness - they might be
helpful for an IDE to understand the DI structure of an application, but
aren't at all used by a downstream compiler.

Generating this metadata is not without cost. When an incremental build
system uses changes in inputs to determine when a rebuild is necessary, any
changes in .d.ts files might cause downstream targets to rebuild. If those
.d.ts changes are in the "private" side of the NgModule (imports or non-
exported directives/pipes), then these rebuilds are wholly unnecessary.

This commit introduces the `onlyPublishPublicTypingsForNgModules` flag for
the compiler. When this flag is set, the compiler will filter the emitted
references in NgModule .d.ts output and only reference those directives/
pipes that are exported from the NgModule (its public API surface). Omitting
the flag preserves the existing behavior of emitting all references, both
public and private.

This is especially useful for build systems such as Bazel.

PR Close #45894

goldens/public-api/compiler-cli/compiler_options.md

@@ -8,6 +8,7 @@
 export interface BazelAndG3Options {
     annotateForClosureCompiler?: boolean;
     generateDeepReexports?: boolean;
+    onlyPublishPublicTypingsForNgModules?: boolean;
 }
 
 // @public

packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_ng_module_linker_1.ts

@@ -45,6 +45,8 @@ export function toR3NgModuleMeta<TExpression>(
     adjacentType: wrappedType,
     bootstrap: [],
     declarations: [],
+    publicDeclarationTypes: null,
+    includeImportTypes: true,
     imports: [],
     exports: [],
     selectorScopeMode: supportJit ? R3SelectorScopeMode.Inline : R3SelectorScopeMode.Omit,

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -141,7 +141,7 @@ export class DecorationAnalyzer {
         this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry,
         this.scopeRegistry, this.referencesRegistry, this.isCore, this.refEmitter,
         /* factoryTracker */ null, !!this.compilerOptions.annotateForClosureCompiler,
-        this.injectableRegistry, NOOP_PERF_RECORDER),
+        /* onlyPublishPublicTypings */ false, this.injectableRegistry, NOOP_PERF_RECORDER),
   ];
   compiler = new NgccTraitCompiler(this.handlers, this.reflectionHost);
   migrations: Migration[] = [

packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts

@@ -131,7 +131,7 @@ export class NgModuleDecoratorHandler implements
       private scopeRegistry: LocalModuleScopeRegistry,
       private referencesRegistry: ReferencesRegistry, private isCore: boolean,
       private refEmitter: ReferenceEmitter, private factoryTracker: FactoryTracker|null,
-      private annotateForClosureCompiler: boolean,
+      private annotateForClosureCompiler: boolean, private onlyPublishPublicTypings: boolean,
       private injectableRegistry: InjectableClassRegistry, private perf: PerfRecorder) {}
 
   readonly precedence = HandlerPrecedence.PRIMARY;
@@ -274,20 +274,32 @@ export class NgModuleDecoratorHandler implements
       typeContext = typeNode.getSourceFile();
     }
 
+
+    const exportedNodes = new Set(exportRefs.map(ref => ref.node));
+    const declarations: R3Reference[] = [];
+    const exportedDeclarations: Expression[] = [];
+
     const bootstrap = bootstrapRefs.map(
         bootstrap => this._toR3Reference(
             bootstrap.getOriginForDiagnostics(meta, node.name), bootstrap, valueContext,
             typeContext));
-    const declarations = declarationRefs.map(
-        decl => this._toR3Reference(
-            decl.getOriginForDiagnostics(meta, node.name), decl, valueContext, typeContext));
+
+    for (const ref of declarationRefs) {
+      const decl = this._toR3Reference(
+          ref.getOriginForDiagnostics(meta, node.name), ref, valueContext, typeContext);
+      declarations.push(decl);
+      if (exportedNodes.has(ref.node)) {
+        exportedDeclarations.push(decl.type);
+      }
+    }
     const imports = importRefs.map(
         imp => this._toR3Reference(
             imp.getOriginForDiagnostics(meta, node.name), imp, valueContext, typeContext));
     const exports = exportRefs.map(
         exp => this._toR3Reference(
             exp.getOriginForDiagnostics(meta, node.name), exp, valueContext, typeContext));
 
+
     const isForwardReference = (ref: R3Reference) =>
         isExpressionForwardReference(ref.value, node.name!, valueContext);
     const containsForwardDecls = bootstrap.some(isForwardReference) ||
@@ -304,8 +316,12 @@ export class NgModuleDecoratorHandler implements
       adjacentType,
       bootstrap,
       declarations,
+      publicDeclarationTypes: this.onlyPublishPublicTypings ? exportedDeclarations : null,
       exports,
       imports,
+      // Imported types are generally private, so include them unless restricting the .d.ts emit to
+      // only public types.
+      includeImportTypes: !this.onlyPublishPublicTypings,
       containsForwardDecls,
       id,
       // Use `ɵɵsetNgModuleScope` to patch selector scopes onto the generated definition in a

packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts

@@ -72,7 +72,8 @@ runInEachFileSystem(() => {
       const handler = new NgModuleDecoratorHandler(
           reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry,
           /* isCore */ false, refEmitter, /* factoryTracker */ null,
-          /* annotateForClosureCompiler */ false, injectableRegistry, NOOP_PERF_RECORDER);
+          /* annotateForClosureCompiler */ false, /* onlyPublishPublicTypings */ false,
+          injectableRegistry, NOOP_PERF_RECORDER);
       const TestModule =
           getDeclaration(program, _('/entry.ts'), 'TestModule', isNamedClassDeclaration);
       const detected =

packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts

@@ -338,6 +338,17 @@ export interface BazelAndG3Options {
    */
   generateDeepReexports?: boolean;
 
+  /**
+   * The `.d.ts` file for NgModules contain type pointers to their declarations, imports, and
+   * exports. Without this flag, the generated type definition will include
+   * components/directives/pipes/NgModules that are declared or imported locally in the NgModule and
+   * not necessarily exported to consumers.
+   *
+   * With this flag set, the type definition generated in the `.d.ts` for an NgModule will be
+   * filtered to only list those types which are publicly exported by the NgModule.
+   */
+  onlyPublishPublicTypingsForNgModules?: boolean;
+
   /**
    * Insert JSDoc type annotations needed by Closure Compiler
    */

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -1044,7 +1044,8 @@ export class NgCompiler {
       new NgModuleDecoratorHandler(
           reflector, evaluator, metaReader, metaRegistry, ngModuleScopeRegistry, referencesRegistry,
           isCore, refEmitter, this.adapter.factoryTracker, this.closureCompilerEnabled,
-          injectableRegistry, this.delegatingPerfRecorder),
+          this.options.onlyPublishPublicTypingsForNgModules ?? false, injectableRegistry,
+          this.delegatingPerfRecorder),
     ];
 
     const traitCompiler = new TraitCompiler(

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -476,6 +476,67 @@ function allTests(os: string) {
       }
     });
 
+    it('should only list public declarations in NgModule .d.ts when requested', () => {
+      env.tsconfig({
+        onlyPublishPublicTypingsForNgModules: true,
+      });
+      env.write('test.ts', `
+            import {Directive, NgModule} from '@angular/core';
+
+            @Directive({selector: 'internal'})
+            export class InternalDir {}
+
+            @Directive({selector: 'external'})
+            export class ExternalDir {}
+
+            @NgModule({
+              declarations: [InternalDir, ExternalDir],
+              exports: [ExternalDir],
+            })
+            export class Module {}
+          `);
+
+      env.driveMain();
+      const dtsContents = env.getContents('test.d.ts');
+      expect(dtsContents)
+          .toContain(
+              'static ɵmod: i0.ɵɵNgModuleDeclaration<Module, ' +
+              '[typeof ExternalDir], never, [typeof ExternalDir]>');
+    });
+
+    it('should not list imports in NgModule .d.ts when requested', () => {
+      env.tsconfig({
+        onlyPublishPublicTypingsForNgModules: true,
+      });
+      env.write('test.ts', `
+            import {Directive, NgModule} from '@angular/core';
+
+            @Directive({selector: 'internal'})
+            export class InternalDir {}
+
+            @NgModule({
+              declarations: [InternalDir],
+              exports: [InternalDir],
+            })
+            export class DepModule {}
+
+            @Directive({selector: 'external'})
+            export class ExternalDir {}
+
+            @NgModule({
+              imports: [DepModule],
+            })
+            export class Module {}
+          `);
+
+      env.driveMain();
+      const dtsContents = env.getContents('test.d.ts');
+      expect(dtsContents)
+          .toContain(
+              'static ɵmod: i0.ɵɵNgModuleDeclaration<Module, ' +
+              'never, never, never>');
+    });
+
     // This test triggers the Tsickle compiler which asserts that the file-paths
     // are valid for the real OS. When on non-Windows systems it doesn't like paths
     // that start with `C:`.
@@ -7328,10 +7389,11 @@ function allTests(os: string) {
            expect(diags.length).toBe(0);
          });
 
-      // TODO(alxhub): this test never worked correctly, as it used to declare a constructor with a
-      // body, which real declaration files don't have. Without the body, the ReflectionHost used to
-      // not return any constructor data, preventing an error from showing. That bug was fixed, but
-      // the error for declaration files is disabled until g3 can be updated.
+      // TODO(alxhub): this test never worked correctly, as it used to declare a constructor
+      // with a body, which real declaration files don't have. Without the body, the
+      // ReflectionHost used to not return any constructor data, preventing an error from
+      // showing. That bug was fixed, but the error for declaration files is disabled until g3
+      // can be updated.
       xit('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass',
           () => {
             env.write('node_modules/@angular/core/testing/index.d.ts', `

packages/compiler/src/jit_compiler_facade.ts

@@ -131,7 +131,9 @@ export class CompilerFacadeImpl implements CompilerFacade {
       adjacentType: new WrappedNodeExpr(facade.type),
       bootstrap: facade.bootstrap.map(wrapReference),
       declarations: facade.declarations.map(wrapReference),
+      publicDeclarationTypes: null,  // only needed for types in AOT
       imports: facade.imports.map(wrapReference),
+      includeImportTypes: true,
       exports: facade.exports.map(wrapReference),
       selectorScopeMode: R3SelectorScopeMode.Inline,
       containsForwardDecls: false,

packages/compiler/src/render3/r3_module_compiler.ts

@@ -81,11 +81,22 @@ export interface R3NgModuleMetadata {
    */
   declarations: R3Reference[];
 
+  /**
+   * Those declarations which should be visible to downstream consumers. If not specified, all
+   * declarations are made visible to downstream consumers.
+   */
+  publicDeclarationTypes: o.Expression[]|null;
+
   /**
    * An array of expressions representing the imports of the module.
    */
   imports: R3Reference[];
 
+  /**
+   * Whether or not to include `imports` in generated type declarations.
+   */
+  includeImportTypes: boolean;
+
   /**
    * An array of expressions representing the exports of the module.
    */
@@ -248,10 +259,14 @@ export function compileNgModuleDeclarationExpression(meta: R3DeclareNgModuleFaca
 }
 
 export function createNgModuleType(
-    {type: moduleType, declarations, imports, exports}: R3NgModuleMetadata): o.ExpressionType {
+    {type: moduleType, declarations, exports, imports, includeImportTypes, publicDeclarationTypes}:
+        R3NgModuleMetadata): o.ExpressionType {
   return new o.ExpressionType(o.importExpr(R3.NgModuleDeclaration, [
-    new o.ExpressionType(moduleType.type), tupleTypeOf(declarations), tupleTypeOf(imports),
-    tupleTypeOf(exports)
+    new o.ExpressionType(moduleType.type),
+    publicDeclarationTypes === null ? tupleTypeOf(declarations) :
+                                      tupleOfTypes(publicDeclarationTypes),
+    includeImportTypes ? tupleTypeOf(imports) : o.NONE_TYPE,
+    tupleTypeOf(exports),
   ]));
 }
 
@@ -308,3 +323,8 @@ function tupleTypeOf(exp: R3Reference[]): o.Type {
   const types = exp.map(ref => o.typeofExpr(ref.type));
   return exp.length > 0 ? o.expressionType(o.literalArr(types)) : o.NONE_TYPE;
 }
+
+function tupleOfTypes(types: o.Expression[]): o.Type {
+  const typeofTypes = types.map(type => o.typeofExpr(type));
+  return types.length > 0 ? o.expressionType(o.literalArr(typeofTypes)) : o.NONE_TYPE;
+}