fix(ivy): prevent ngcc from referencing missing ɵsetClassMetadata (#27055)

When ngtsc compiles @angular/core, it rewrites core imports to the
r3_symbols.ts file that exposes all internal symbols under their
external name. When creating the FESM bundle, the r3_symbols.ts file
causes the external symbol names to be rewritten to their internal name.

Under ngcc compilations of FESM bundles, the indirection of
r3_symbols.ts is no longer in place such that the external names are
retained in the bundle. Previously, the external name `ɵdefineNgModule`
was explicitly declared internally to resolve this issue, but the
recently added `setClassMetadata` was not declared as such, causing
runtime errors.

Instead of relying on the r3_symbols.ts file to perform the rewrite of
the external modules to their internal variants, the translation is
moved into the `ImportManager` during the compilation itself. This
avoids the need for providing the external name manually.

PR Close #27055

integration/ngcc/test.sh

@@ -24,13 +24,13 @@ ivy-ngcc
   if [[ $? != 0 ]]; then exit 1; fi
 
 # Did it compile @angular/core/ApplicationModule correctly?
-  grep "ApplicationModule.ngModuleDef = ɵdefineNgModule" node_modules/@angular/core/fesm2015/core.js
+  grep "ApplicationModule.ngModuleDef = defineNgModule" node_modules/@angular/core/fesm2015/core.js
   if [[ $? != 0 ]]; then exit 1; fi
-  grep "ApplicationModule.ngModuleDef = ɵdefineNgModule" node_modules/@angular/core/fesm5/core.js
+  grep "ApplicationModule.ngModuleDef = defineNgModule" node_modules/@angular/core/fesm5/core.js
   if [[ $? != 0 ]]; then exit 1; fi
-  grep "ApplicationModule.ngModuleDef = ɵngcc0.ɵdefineNgModule" node_modules/@angular/core/esm2015/src/application_module.js
+  grep "ApplicationModule.ngModuleDef = ɵngcc0.defineNgModule" node_modules/@angular/core/esm2015/src/application_module.js
   if [[ $? != 0 ]]; then exit 1; fi
-  grep "ApplicationModule.ngModuleDef = ɵngcc0.ɵdefineNgModule" node_modules/@angular/core/esm5/src/application_module.js
+  grep "ApplicationModule.ngModuleDef = ɵngcc0.defineNgModule" node_modules/@angular/core/esm5/src/application_module.js
   if [[ $? != 0 ]]; then exit 1; fi
 
 # Can it be safely run again (as a noop)?

packages/compiler-cli/src/ngcc/src/rendering/ngcc_import_manager.ts

@@ -12,9 +12,10 @@ import {ImportManager} from '../../../ngtsc/translator';
 export class NgccImportManager extends ImportManager {
   constructor(private isFlat: boolean, isCore: boolean, prefix?: string) { super(isCore, prefix); }
 
-  generateNamedImport(moduleName: string, symbol: string): string|null {
+  generateNamedImport(moduleName: string, symbol: string):
+      {moduleImport: string | null, symbol: string} {
     if (this.isFlat && this.isCore && moduleName === '@angular/core') {
-      return null;
+      return {moduleImport: null, symbol: this.rewriteSymbol(moduleName, symbol)};
     }
     return super.generateNamedImport(moduleName, symbol);
   }

packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts

@@ -17,7 +17,10 @@ import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
 import {Renderer} from '../../src/rendering/renderer';
 
 class TestRenderer extends Renderer {
-  constructor(host: Esm2015ReflectionHost) { super(host, false, null, '/src', '/dist', false); }
+  constructor(
+      host: Esm2015ReflectionHost, isCore: boolean, rewriteCoreImportsTo: ts.SourceFile|null) {
+    super(host, isCore, rewriteCoreImportsTo, '/src', '/dist', false);
+  }
   addImports(output: MagicString, imports: {name: string, as: string}[]) {
     output.prepend('\n// ADD IMPORTS\n');
   }
@@ -35,13 +38,18 @@ class TestRenderer extends Renderer {
   }
 }
 
-function createTestRenderer(file: {name: string, contents: string}) {
-  const program = makeProgram(file);
-  const host = new Esm2015ReflectionHost(false, program.getTypeChecker());
+function createTestRenderer(
+    files: {name: string, contents: string}[],
+    options: {isCore?: boolean, rewriteCoreImportsTo?: string} = {}) {
+  const program = makeProgram(...files);
+  const host = new Esm2015ReflectionHost(options.isCore || false, program.getTypeChecker());
   const decorationAnalyses =
-      new DecorationAnalyzer(program.getTypeChecker(), host, [''], false).analyzeProgram(program);
+      new DecorationAnalyzer(program.getTypeChecker(), host, [''], options.isCore || false)
+          .analyzeProgram(program);
   const switchMarkerAnalyses = new SwitchMarkerAnalyzer(host).analyzeProgram(program);
-  const renderer = new TestRenderer(host);
+  const rewriteCoreImportsTo =
+      options.rewriteCoreImportsTo ? program.getSourceFile(options.rewriteCoreImportsTo) ! : null;
+  const renderer = new TestRenderer(host, options.isCore || false, rewriteCoreImportsTo);
   spyOn(renderer, 'addImports').and.callThrough();
   spyOn(renderer, 'addDefinitions').and.callThrough();
   spyOn(renderer, 'removeDecorators').and.callThrough();
@@ -94,7 +102,7 @@ describe('Renderer', () => {
     it('should render the modified contents; and a new map file, if the original provided no map file.',
        () => {
          const {renderer, program, decorationAnalyses, switchMarkerAnalyses} =
-             createTestRenderer(INPUT_PROGRAM);
+             createTestRenderer([INPUT_PROGRAM]);
          const result = renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          expect(result[0].path).toEqual('/dist/file.js');
          expect(result[0].contents)
@@ -106,7 +114,7 @@ describe('Renderer', () => {
     it('should call addImports with the source code and info about the core Angular library.',
        () => {
          const {decorationAnalyses, program, renderer, switchMarkerAnalyses} =
-             createTestRenderer(INPUT_PROGRAM);
+             createTestRenderer([INPUT_PROGRAM]);
          renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          const addImportsSpy = renderer.addImports as jasmine.Spy;
          expect(addImportsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
@@ -118,7 +126,7 @@ describe('Renderer', () => {
     it('should call addDefinitions with the source code, the analyzed class and the renderered definitions.',
        () => {
          const {decorationAnalyses, program, renderer, switchMarkerAnalyses} =
-             createTestRenderer(INPUT_PROGRAM);
+             createTestRenderer([INPUT_PROGRAM]);
          renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          const addDefinitionsSpy = renderer.addDefinitions as jasmine.Spy;
          expect(addDefinitionsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
@@ -137,7 +145,7 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
     it('should call removeDecorators with the source code, a map of class decorators that have been analyzed',
        () => {
          const {decorationAnalyses, program, renderer, switchMarkerAnalyses} =
-             createTestRenderer(INPUT_PROGRAM);
+             createTestRenderer([INPUT_PROGRAM]);
          renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          const removeDecoratorsSpy = renderer.removeDecorators as jasmine.Spy;
          expect(removeDecoratorsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
@@ -157,10 +165,10 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
 
     it('should merge any inline source map from the original file and write the output as an inline source map',
        () => {
-         const {decorationAnalyses, program, renderer, switchMarkerAnalyses} = createTestRenderer({
+         const {decorationAnalyses, program, renderer, switchMarkerAnalyses} = createTestRenderer([{
            ...INPUT_PROGRAM,
            contents: INPUT_PROGRAM.contents + '\n' + INPUT_PROGRAM_MAP.toComment()
-         });
+         }]);
          const result = renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          expect(result[0].path).toEqual('/dist/file.js');
          expect(result[0].contents)
@@ -172,10 +180,10 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
        () => {
          // Mock out reading the map file from disk
          spyOn(fs, 'readFileSync').and.returnValue(INPUT_PROGRAM_MAP.toJSON());
-         const {decorationAnalyses, program, renderer, switchMarkerAnalyses} = createTestRenderer({
+         const {decorationAnalyses, program, renderer, switchMarkerAnalyses} = createTestRenderer([{
            ...INPUT_PROGRAM,
            contents: INPUT_PROGRAM.contents + '\n//# sourceMappingURL=file.js.map'
-         });
+         }]);
          const result = renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
          expect(result[0].path).toEqual('/dist/file.js');
          expect(result[0].contents)
@@ -183,5 +191,48 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
          expect(result[1].path).toEqual('/dist/file.js.map');
          expect(result[1].contents).toEqual(MERGED_OUTPUT_PROGRAM_MAP.toJSON());
        });
+
+    describe('@angular/core support', () => {
+
+      it('should render relative imports in ESM bundles', () => {
+        const R3_SYMBOLS_FILE = {
+          name: '/src/r3_symbols.js',
+          contents: `export const NgModule = () => null;`
+        };
+        const CORE_FILE = {
+          name: '/src/core.js',
+          contents:
+              `import { NgModule } from './ng_module';\nexport class MyModule {}\nMyModule.decorators = [\n    { type: NgModule, args: [] }\n];\n`
+        };
+
+        const {decorationAnalyses, program, renderer, switchMarkerAnalyses} = createTestRenderer(
+            [R3_SYMBOLS_FILE, CORE_FILE],
+            {isCore: true, rewriteCoreImportsTo: R3_SYMBOLS_FILE.name});
+        renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
+        const addDefinitionsSpy = renderer.addDefinitions as jasmine.Spy;
+        expect(addDefinitionsSpy.calls.first().args[2])
+            .toContain(`/*@__PURE__*/ ɵngcc0.setClassMetadata(`);
+        const addImportsSpy = renderer.addImports as jasmine.Spy;
+        expect(addImportsSpy.calls.first().args[1]).toEqual([{name: './r3_symbols', as: 'ɵngcc0'}]);
+      });
+
+      it('should render no imports in FESM bundles', () => {
+        const CORE_FILE = {
+          name: '/src/core.js',
+          contents: `export const NgModule = () => null;
+            export class MyModule {}\nMyModule.decorators = [\n    { type: NgModule, args: [] }\n];\n`
+        };
+
+        const {decorationAnalyses, program, renderer, switchMarkerAnalyses} =
+            createTestRenderer([CORE_FILE], {isCore: true});
+        renderer.renderProgram(program, decorationAnalyses, switchMarkerAnalyses);
+        const addDefinitionsSpy = renderer.addDefinitions as jasmine.Spy;
+        expect(addDefinitionsSpy.calls.first().args[2])
+            .toContain(`/*@__PURE__*/ setClassMetadata(`);
+        const addImportsSpy = renderer.addImports as jasmine.Spy;
+        expect(addImportsSpy.calls.first().args[1]).toEqual([]);
+      });
+
+    });
   });
 });

packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts

@@ -17,7 +17,7 @@ import * as ts from 'typescript';
 
 import {ClassMemberKind, ReflectionHost} from '../../host';
 
-const TS_DTS_JS_EXTENSION = /(\.d)?\.ts|\.js$/;
+const TS_DTS_JS_EXTENSION = /(?:\.d)?\.ts$|\.js$/;
 
 /**
  * Represents a value which cannot be determined statically.

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

@@ -38,16 +38,16 @@ const BINARY_OPERATORS = new Map<BinaryOperator, ts.BinaryOperator>([
   [BinaryOperator.Plus, ts.SyntaxKind.PlusToken],
 ]);
 
-const CORE_SUPPORTED_SYMBOLS = new Set<string>([
-  'defineInjectable',
-  'defineInjector',
-  'ɵdefineNgModule',
-  'inject',
-  'ɵsetClassMetadata',
-  'ɵInjectableDef',
-  'ɵInjectorDef',
-  'ɵNgModuleDefWithMeta',
-  'ɵNgModuleFactory',
+const CORE_SUPPORTED_SYMBOLS = new Map<string, string>([
+  ['defineInjectable', 'defineInjectable'],
+  ['defineInjector', 'defineInjector'],
+  ['ɵdefineNgModule', 'defineNgModule'],
+  ['inject', 'inject'],
+  ['ɵsetClassMetadata', 'setClassMetadata'],
+  ['ɵInjectableDef', 'InjectableDef'],
+  ['ɵInjectorDef', 'InjectorDef'],
+  ['ɵNgModuleDefWithMeta', 'NgModuleDefWithMeta'],
+  ['ɵNgModuleFactory', 'NgModuleFactory'],
 ]);
 
 export class ImportManager {
@@ -56,14 +56,28 @@ export class ImportManager {
 
   constructor(protected isCore: boolean, private prefix = 'i') {}
 
-  generateNamedImport(moduleName: string, symbol: string): string|null {
+  generateNamedImport(moduleName: string, symbol: string):
+      {moduleImport: string | null, symbol: string} {
     if (!this.moduleToIndex.has(moduleName)) {
       this.moduleToIndex.set(moduleName, `${this.prefix}${this.nextIndex++}`);
     }
-    if (this.isCore && moduleName === '@angular/core' && !CORE_SUPPORTED_SYMBOLS.has(symbol)) {
-      throw new Error(`Importing unexpected symbol ${symbol} while compiling core`);
+
+    return {
+      moduleImport: this.moduleToIndex.get(moduleName) !,
+      symbol: this.rewriteSymbol(moduleName, symbol)
+    };
+  }
+
+  protected rewriteSymbol(moduleName: string, symbol: string): string {
+    if (this.isCore && moduleName === '@angular/core') {
+      if (!CORE_SUPPORTED_SYMBOLS.has(symbol)) {
+        throw new Error(`Importing unexpected symbol ${symbol} while compiling core`);
+      }
+
+      symbol = CORE_SUPPORTED_SYMBOLS.get(symbol) !;
     }
-    return this.moduleToIndex.get(moduleName) !;
+
+    return symbol;
   }
 
   getAllImports(contextPath: string, rewriteCoreImportsTo: ts.SourceFile|null):
@@ -216,12 +230,13 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
     if (ast.value.moduleName === null || ast.value.name === null) {
       throw new Error(`Import unknown module or symbol ${ast.value}`);
     }
-    const importIdentifier = this.imports.generateNamedImport(ast.value.moduleName, ast.value.name);
-    if (importIdentifier === null) {
-      return ts.createIdentifier(ast.value.name);
+    const {moduleImport, symbol} =
+        this.imports.generateNamedImport(ast.value.moduleName, ast.value.name);
+    if (moduleImport === null) {
+      return ts.createIdentifier(symbol);
     } else {
       return ts.createPropertyAccess(
-          ts.createIdentifier(importIdentifier), ts.createIdentifier(ast.value.name));
+          ts.createIdentifier(moduleImport), ts.createIdentifier(symbol));
     }
   }
 
@@ -382,8 +397,9 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor {
     if (ast.value.moduleName === null || ast.value.name === null) {
       throw new Error(`Import unknown module or symbol`);
     }
-    const moduleSymbol = this.imports.generateNamedImport(ast.value.moduleName, ast.value.name);
-    const base = `${moduleSymbol}.${ast.value.name}`;
+    const {moduleImport, symbol} =
+        this.imports.generateNamedImport(ast.value.moduleName, ast.value.name);
+    const base = moduleImport ? `${moduleImport}.${symbol}` : symbol;
     if (ast.typeParams !== null) {
       const generics = ast.typeParams.map(type => type.visitType(this, context)).join(', ');
       return `${base}<${generics}>`;

packages/compiler-cli/src/ngtsc/util/src/path.ts

@@ -10,10 +10,10 @@
 
 import * as path from 'path';
 
-const TS_DTS_EXTENSION = /(\.d)?\.ts$/;
+const TS_DTS_JS_EXTENSION = /(?:\.d)?\.ts$|\.js$/;
 
 export function relativePathBetween(from: string, to: string): string|null {
-  let relative = path.posix.relative(path.dirname(from), to).replace(TS_DTS_EXTENSION, '');
+  let relative = path.posix.relative(path.dirname(from), to).replace(TS_DTS_JS_EXTENSION, '');
 
   if (relative === '') {
     return null;

packages/core/src/core_render3_private_export.ts

@@ -7,15 +7,12 @@
  */
 
 // clang-format off
-// We need to have `ɵdefineNgModule` defined locally for flat-file ngcc compilation.
-// More details in the commit where this is added.
-import {defineNgModule} from './render3/index';
-export const ɵdefineNgModule = defineNgModule;
 export {
   defineBase as ɵdefineBase,
   defineComponent as ɵdefineComponent,
   defineDirective as ɵdefineDirective,
   definePipe as ɵdefinePipe,
+  defineNgModule as ɵdefineNgModule,
   detectChanges as ɵdetectChanges,
   renderComponent as ɵrenderComponent,
   ComponentType as ɵComponentType,

packages/core/src/r3_symbols.ts

@@ -14,17 +14,18 @@
  * compiler writes imports to this file.
  *
  * Only a subset of such imports are supported - core is not allowed to declare components or pipes.
- * A check in ngtsc's translator.ts validates this condition.
+ * A check in ngtsc's translator.ts validates this condition. The translator is responsible for
+ * translating an external name (prefixed with ɵ) to the internal symbol name as exported below.
  *
  * The below symbols are used for @Injectable and @NgModule compilation.
  */
 
-export {InjectableDef as ɵInjectableDef, InjectorDef as ɵInjectorDef, defineInjectable, defineInjector} from './di/defs';
+export {InjectableDef, InjectorDef, defineInjectable, defineInjector} from './di/defs';
 export {inject} from './di/injector_compatibility';
-export {NgModuleDef as ɵNgModuleDef, NgModuleDefWithMeta as ɵNgModuleDefWithMeta} from './metadata/ng_module';
-export {defineNgModule as ɵdefineNgModule} from './render3/definition';
-export {setClassMetadata as ɵsetClassMetadata} from './render3/metadata';
-export {NgModuleFactory as ɵNgModuleFactory} from './render3/ng_module_ref';
+export {NgModuleDef, NgModuleDefWithMeta} from './metadata/ng_module';
+export {defineNgModule} from './render3/definition';
+export {setClassMetadata} from './render3/metadata';
+export {NgModuleFactory} from './render3/ng_module_ref';
 
 
 /**

packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json

@@ -1420,8 +1420,5 @@
   },
   {
     "name": "wtfLeave"
-  },
-  {
-    "name": "ɵdefineNgModule"
   }
 ]

packages/core/test/bundling/todo_r2/bundle.golden_symbols.json

@@ -2704,8 +2704,5 @@
   },
   {
     "name": "wtfLeave"
-  },
-  {
-    "name": "ɵdefineNgModule"
   }
 ]