fix(ivy): properly rewrite imports in generated factory shims (#27998)

Generated factory shims can import from @angular/core. However, we have
special logic in place to rewrite self-imports when generating code for
@angular/core.

This commit leverages the new standalone ImportRewriter interface to
properly rewrite imports in generated factory shims. Before this fix,
a generated factory file for core would look like:

```typescript
import * as i0 from './r3_symbols';

export var ApplicationModuleNgFactory = new ɵNgModuleFactory(...);
```

This is invalid, as ɵNgModuleFactory is just NgModuleFactory when imported
via r3_symbols.

FW-881 #resolve

PR Close #27998
This commit is contained in:
Alex Rickabaugh 2019-01-08 13:02:11 -08:00 committed by Andrew Kushnir
parent 3cf1b62722
commit 6003145422
4 changed files with 80 additions and 14 deletions

View File

@ -239,7 +239,7 @@ export class NgtscProgram implements api.Program {
if (this.factoryToSourceInfo !== null) { if (this.factoryToSourceInfo !== null) {
beforeTransforms.push( beforeTransforms.push(
generatedFactoryTransform(this.factoryToSourceInfo, this.coreImportsFrom)); generatedFactoryTransform(this.factoryToSourceInfo, this.importRewriter));
} }
if (this.isCore) { if (this.isCore) {
beforeTransforms.push(ivySwitchTransform); beforeTransforms.push(ivySwitchTransform);

View File

@ -11,6 +11,7 @@ ts_library(
module_name = "@angular/compiler-cli/src/ngtsc/shims", module_name = "@angular/compiler-cli/src/ngtsc/shims",
deps = [ deps = [
"//packages/compiler", "//packages/compiler",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/src/ngtsc/util",
"@ngdeps//@types/node", "@ngdeps//@types/node",
"@ngdeps//typescript", "@ngdeps//typescript",

View File

@ -9,7 +9,7 @@
import * as path from 'path'; import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {relativePathBetween} from '../../util/src/path'; import {ImportRewriter} from '../../imports';
import {isNonDeclarationTsPath} from '../../util/src/typescript'; import {isNonDeclarationTsPath} from '../../util/src/typescript';
import {ShimGenerator} from './host'; import {ShimGenerator} from './host';
@ -106,17 +106,17 @@ export interface FactoryInfo {
export function generatedFactoryTransform( export function generatedFactoryTransform(
factoryMap: Map<string, FactoryInfo>, factoryMap: Map<string, FactoryInfo>,
coreImportsFrom: ts.SourceFile | null): ts.TransformerFactory<ts.SourceFile> { importRewriter: ImportRewriter): ts.TransformerFactory<ts.SourceFile> {
return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => { return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
return (file: ts.SourceFile): ts.SourceFile => { return (file: ts.SourceFile): ts.SourceFile => {
return transformFactorySourceFile(factoryMap, context, coreImportsFrom, file); return transformFactorySourceFile(factoryMap, context, importRewriter, file);
}; };
}; };
} }
function transformFactorySourceFile( function transformFactorySourceFile(
factoryMap: Map<string, FactoryInfo>, context: ts.TransformationContext, factoryMap: Map<string, FactoryInfo>, context: ts.TransformationContext,
coreImportsFrom: ts.SourceFile | null, file: ts.SourceFile): ts.SourceFile { importRewriter: ImportRewriter, file: ts.SourceFile): ts.SourceFile {
// If this is not a generated file, it won't have factory info associated with it. // If this is not a generated file, it won't have factory info associated with it.
if (!factoryMap.has(file.fileName)) { if (!factoryMap.has(file.fileName)) {
// Don't transform non-generated code. // Don't transform non-generated code.
@ -125,7 +125,7 @@ function transformFactorySourceFile(
const {moduleSymbolNames, sourceFilePath} = factoryMap.get(file.fileName) !; const {moduleSymbolNames, sourceFilePath} = factoryMap.get(file.fileName) !;
const clone = ts.getMutableClone(file); file = ts.getMutableClone(file);
// Not every exported factory statement is valid. They were generated before the program was // Not every exported factory statement is valid. They were generated before the program was
// analyzed, and before ngtsc knew which symbols were actually NgModules. factoryMap contains // analyzed, and before ngtsc knew which symbols were actually NgModules. factoryMap contains
@ -146,17 +146,30 @@ function transformFactorySourceFile(
// The statement identified as the ɵNonEmptyModule export. // The statement identified as the ɵNonEmptyModule export.
let nonEmptyExport: ts.Statement|null = null; let nonEmptyExport: ts.Statement|null = null;
// Extracted identifiers which refer to import statements from @angular/core.
const coreImportIdentifiers = new Set<string>();
// Consider all the statements. // Consider all the statements.
for (const stmt of file.statements) { for (const stmt of file.statements) {
// Look for imports to @angular/core. // Look for imports to @angular/core.
if (coreImportsFrom !== null && ts.isImportDeclaration(stmt) && if (ts.isImportDeclaration(stmt) && ts.isStringLiteral(stmt.moduleSpecifier) &&
ts.isStringLiteral(stmt.moduleSpecifier) && stmt.moduleSpecifier.text === '@angular/core') { stmt.moduleSpecifier.text === '@angular/core') {
// Update the import path to point to the correct file (coreImportsFrom). // Update the import path to point to the correct file using the ImportRewriter.
const path = relativePathBetween(sourceFilePath, coreImportsFrom.fileName); const rewrittenModuleSpecifier =
if (path !== null) { importRewriter.rewriteSpecifier('@angular/core', sourceFilePath);
if (rewrittenModuleSpecifier !== stmt.moduleSpecifier.text) {
transformedStatements.push(ts.updateImportDeclaration( transformedStatements.push(ts.updateImportDeclaration(
stmt, stmt.decorators, stmt.modifiers, stmt.importClause, stmt, stmt.decorators, stmt.modifiers, stmt.importClause,
ts.createStringLiteral(path))); ts.createStringLiteral(rewrittenModuleSpecifier)));
// Record the identifier by which this imported module goes, so references to its symbols
// can be discovered later.
if (stmt.importClause !== undefined && stmt.importClause.namedBindings !== undefined &&
ts.isNamespaceImport(stmt.importClause.namedBindings)) {
coreImportIdentifiers.add(stmt.importClause.namedBindings.name.text);
}
} else {
transformedStatements.push(stmt);
} }
} else if (ts.isVariableStatement(stmt) && stmt.declarationList.declarations.length === 1) { } else if (ts.isVariableStatement(stmt) && stmt.declarationList.declarations.length === 1) {
const decl = stmt.declarationList.declarations[0]; const decl = stmt.declarationList.declarations[0];
@ -189,6 +202,31 @@ function transformFactorySourceFile(
// satisfy closure compiler. // satisfy closure compiler.
transformedStatements.push(nonEmptyExport); transformedStatements.push(nonEmptyExport);
} }
clone.statements = ts.createNodeArray(transformedStatements); file.statements = ts.createNodeArray(transformedStatements);
return clone;
// If any imports to @angular/core were detected and rewritten (which happens when compiling
// @angular/core), go through the SourceFile and rewrite references to symbols imported from core.
if (coreImportIdentifiers.size > 0) {
const visit = <T extends ts.Node>(node: T): T => {
node = ts.visitEachChild(node, child => visit(child), context);
// Look for expressions of the form "i.s" where 'i' is a detected name for an @angular/core
// import that was changed above. Rewrite 's' using the ImportResolver.
if (ts.isPropertyAccessExpression(node) && ts.isIdentifier(node.expression) &&
coreImportIdentifiers.has(node.expression.text)) {
// This is an import of a symbol from @angular/core. Transform it with the importRewriter.
const rewrittenSymbol = importRewriter.rewriteSymbol(node.name.text, '@angular/core');
if (rewrittenSymbol !== node.name.text) {
const updated =
ts.updatePropertyAccess(node, node.expression, ts.createIdentifier(rewrittenSymbol));
node = updated as T & ts.PropertyAccessExpression;
}
}
return node;
};
file = visit(file);
}
return file;
} }

View File

@ -905,6 +905,29 @@ describe('ngtsc behavioral tests', () => {
expect(emptyFactory).toContain(`export var ɵNonEmptyModule = true;`); expect(emptyFactory).toContain(`export var ɵNonEmptyModule = true;`);
}); });
it('should generate correct imports in factory stubs when compiling @angular/core', () => {
env.tsconfig({'allowEmptyCodegenFiles': true});
env.write('test.ts', `
import {NgModule} from '@angular/core';
@NgModule({})
export class TestModule {}
`);
// Trick the compiler into thinking it's compiling @angular/core.
env.write('r3_symbols.ts', 'export const ITS_JUST_ANGULAR = true;');
env.driveMain();
const factoryContents = env.getContents('test.ngfactory.js');
expect(normalize(factoryContents)).toBe(normalize(`
import * as i0 from "./r3_symbols";
import { TestModule } from './test';
export var TestModuleNgFactory = new i0.NgModuleFactory(TestModule);
`));
});
it('should generate a summary stub for decorated classes in the input file only', () => { it('should generate a summary stub for decorated classes in the input file only', () => {
env.tsconfig({'allowEmptyCodegenFiles': true}); env.tsconfig({'allowEmptyCodegenFiles': true});
@ -1635,3 +1658,7 @@ function expectTokenAtPosition<T extends ts.Node>(
expect(guard(node)).toBe(true); expect(guard(node)).toBe(true);
return node as T; return node as T;
} }
function normalize(input: string): string {
return input.replace(/\s+/g, ' ').trim();
}