fix(ivy): add flag to skip non-exported classes

In ViewEngine we were only generating code for exported classes, however with Ivy we do it no matter whether the class has been exported or not. These changes add an extra flag that allows consumers to opt into the ViewEngine behavior. The flag works by treating non-exported classes as if they're set to `jit: true`. Fixes angular#33724.
crisbeto · Nov 21, 2019 · a905e08 · a905e08
1 parent 6bf2531
commit a905e08
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 3 deletions.
diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts
@@ -607,7 +607,8 @@ export class NgtscProgram implements api.Program {
 
     return new IvyCompilation(
         handlers, this.reflector, this.importRewriter, this.incrementalState, this.perfRecorder,
-        this.sourceToFactorySymbols, scopeRegistry);
+        this.sourceToFactorySymbols, scopeRegistry,
+        this.options.compileNonExportedClasses !== false);
   }
 
   private getI18nLegacyMessageFormat(): string {

diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts
@@ -17,7 +17,7 @@ import {PerfRecorder} from '../../perf';
 import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration, reflectNameOfDeclaration} from '../../reflection';
 import {LocalModuleScopeRegistry} from '../../scope';
 import {TypeCheckContext} from '../../typecheck';
-import {getSourceFile} from '../../util/src/typescript';
+import {getSourceFile, isExported} from '../../util/src/typescript';
 
 import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from './api';
 import {DtsFileTransformer} from './declaration';
@@ -80,7 +80,8 @@ export class IvyCompilation {
       private handlers: DecoratorHandler<any, any>[], private reflector: ReflectionHost,
       private importRewriter: ImportRewriter, private incrementalState: IncrementalState,
       private perf: PerfRecorder, private sourceToFactorySymbols: Map<string, Set<string>>|null,
-      private scopeRegistry: LocalModuleScopeRegistry) {}
+      private scopeRegistry: LocalModuleScopeRegistry, private compileNonExportedClasses: boolean) {
+  }
 
 
   get exportStatements(): Map<string, Map<string, [string, string]>> { return this.reexportMap; }
@@ -90,6 +91,10 @@ export class IvyCompilation {
   analyzeAsync(sf: ts.SourceFile): Promise<void>|undefined { return this.analyze(sf, true); }
 
   private detectHandlersForClass(node: ClassDeclaration): IvyClass|null {
+    if (!this.compileNonExportedClasses && !isExported(node)) {
+      return null;
+    }
+
     // The first step is to reflect the decorators.
     const classDecorators = this.reflector.getDecoratorsOfDeclaration(node);
     let ivyClass: IvyClass|null = null;

diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts
@@ -392,6 +392,12 @@ export interface CompilerOptions extends ts.CompilerOptions {
    * support these future imports.
    */
   generateDeepReexports?: boolean;
+
+  /**
+   * Whether the compiler should avoid generating code for classes that haven't been exported.
+   * This is only active when building with `enableIvy: true`. Defaults to `true`.
+   */
+  compileNonExportedClasses?: boolean;
 }
 
 export interface CompilerHost extends ts.CompilerHost {

diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -5021,6 +5021,60 @@ export const Foo = Foo__PRE_R3__;
         expect(jsContents).toContain('styles: ["h1[_ngcontent-%COMP%] {font-size: larger}"]');
       });
     });
+
+    describe('non-exported classes', () => {
+      beforeEach(() => env.tsconfig({compileNonExportedClasses: false}));
+
+      it('should not emit directive definitions for non-exported classes if configured', () => {
+        env.write('test.ts', `
+          import {Directive} from '@angular/core';
+
+          @Directive({
+            selector: '[test]'
+          })
+          class TestDirective {}
+        `);
+        env.driveMain();
+        const jsContents = env.getContents('test.js');
+
+        expect(jsContents).not.toContain('defineDirective(');
+        expect(jsContents).toContain('Directive({');
+      });
+
+      it('should not emit component definitions for non-exported classes if configured', () => {
+        env.write('test.ts', `
+          import {Component} from '@angular/core';
+
+          @Component({
+            selector: 'test',
+            template: 'hello'
+          })
+          class TestComponent {}
+        `);
+        env.driveMain();
+        const jsContents = env.getContents('test.js');
+
+        expect(jsContents).not.toContain('defineComponent(');
+        expect(jsContents).toContain('Component({');
+      });
+
+      it('should not emit module definitions for non-exported classes if configured', () => {
+        env.write('test.ts', `
+          import {NgModule} from '@angular/core';
+
+          @NgModule({
+            declarations: []
+          })
+          class TestModule {}
+        `);
+        env.driveMain();
+        const jsContents = env.getContents('test.js');
+
+        expect(jsContents).not.toContain('defineNgModule(');
+        expect(jsContents).toContain('NgModule({');
+      });
+    });
+
   });
 
   function expectTokenAtPosition<T extends ts.Node>(