fix(ivy): properly compile NgModules with forward referenced types (#29198)
Previously, ngtsc would resolve forward references while evaluating the bootstrap, declaration, imports, and exports fields of NgModule types. However, when generating the resulting ngModuleDef, the forward nature of these references was not taken into consideration, and so the generated JS code would incorrectly reference types not yet declared. This commit fixes this issue by introducing function closures in the NgModuleDef type, similarly to how NgComponentDef uses them for forward declarations of its directives and pipes arrays. ngtsc will then generate closures when required, and the runtime will unwrap them if present. PR Close #29198
This commit is contained in:

committed by
Kara Erickson

parent
1625d86178
commit
73da2792c9
@ -23,7 +23,7 @@ import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
|
||||
import {ResourceLoader} from './api';
|
||||
import {extractDirectiveMetadata, extractQueriesFromDecorator, parseFieldArrayValue, queriesFromFields} from './directive';
|
||||
import {generateSetClassMetadataCall} from './metadata';
|
||||
import {findAngularDecorator, isAngularCoreReference, unwrapExpression} from './util';
|
||||
import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, unwrapExpression} from './util';
|
||||
|
||||
const EMPTY_MAP = new Map<string, Expression>();
|
||||
const EMPTY_ARRAY: any[] = [];
|
||||
@ -616,20 +616,6 @@ function getTemplateRange(templateExpr: ts.Expression) {
|
||||
};
|
||||
}
|
||||
|
||||
function isExpressionForwardReference(
|
||||
expr: Expression, context: ts.Node, contextSource: ts.SourceFile): boolean {
|
||||
if (isWrappedTsNodeExpr(expr)) {
|
||||
const node = ts.getOriginalNode(expr.node);
|
||||
return node.getSourceFile() === contextSource && context.pos < node.pos;
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function isWrappedTsNodeExpr(expr: Expression): expr is WrappedNodeExpr<ts.Node> {
|
||||
return expr instanceof WrappedNodeExpr;
|
||||
}
|
||||
|
||||
function sourceMapUrl(resourceUrl: string): string {
|
||||
if (!tsSourceMapBug29300Fixed()) {
|
||||
// By removing the template URL we are telling the translator not to try to
|
||||
|
@ -20,7 +20,7 @@ import {getSourceFile} from '../../util/src/typescript';
|
||||
|
||||
import {generateSetClassMetadataCall} from './metadata';
|
||||
import {ReferencesRegistry} from './references_registry';
|
||||
import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, toR3Reference, unwrapExpression} from './util';
|
||||
import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression} from './util';
|
||||
|
||||
export interface NgModuleAnalysis {
|
||||
ngModuleDef: R3NgModuleMetadata;
|
||||
@ -90,38 +90,39 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
||||
]);
|
||||
|
||||
// Extract the module declarations, imports, and exports.
|
||||
let declarations: Reference<ts.Declaration>[] = [];
|
||||
let declarationRefs: Reference<ts.Declaration>[] = [];
|
||||
if (ngModule.has('declarations')) {
|
||||
const expr = ngModule.get('declarations') !;
|
||||
const declarationMeta = this.evaluator.evaluate(expr, forwardRefResolver);
|
||||
declarations = this.resolveTypeList(expr, declarationMeta, name, 'declarations');
|
||||
declarationRefs = this.resolveTypeList(expr, declarationMeta, name, 'declarations');
|
||||
}
|
||||
let imports: Reference<ts.Declaration>[] = [];
|
||||
let importRefs: Reference<ts.Declaration>[] = [];
|
||||
let rawImports: ts.Expression|null = null;
|
||||
if (ngModule.has('imports')) {
|
||||
rawImports = ngModule.get('imports') !;
|
||||
const importsMeta = this.evaluator.evaluate(rawImports, moduleResolvers);
|
||||
imports = this.resolveTypeList(rawImports, importsMeta, name, 'imports');
|
||||
importRefs = this.resolveTypeList(rawImports, importsMeta, name, 'imports');
|
||||
}
|
||||
let exports: Reference<ts.Declaration>[] = [];
|
||||
let exportRefs: Reference<ts.Declaration>[] = [];
|
||||
let rawExports: ts.Expression|null = null;
|
||||
if (ngModule.has('exports')) {
|
||||
rawExports = ngModule.get('exports') !;
|
||||
const exportsMeta = this.evaluator.evaluate(rawExports, moduleResolvers);
|
||||
exports = this.resolveTypeList(rawExports, exportsMeta, name, 'exports');
|
||||
this.referencesRegistry.add(node, ...exports);
|
||||
exportRefs = this.resolveTypeList(rawExports, exportsMeta, name, 'exports');
|
||||
this.referencesRegistry.add(node, ...exportRefs);
|
||||
}
|
||||
let bootstrap: Reference<ts.Declaration>[] = [];
|
||||
let bootstrapRefs: Reference<ts.Declaration>[] = [];
|
||||
if (ngModule.has('bootstrap')) {
|
||||
const expr = ngModule.get('bootstrap') !;
|
||||
const bootstrapMeta = this.evaluator.evaluate(expr, forwardRefResolver);
|
||||
bootstrap = this.resolveTypeList(expr, bootstrapMeta, name, 'bootstrap');
|
||||
bootstrapRefs = this.resolveTypeList(expr, bootstrapMeta, name, 'bootstrap');
|
||||
}
|
||||
|
||||
// Register this module's information with the LocalModuleScopeRegistry. This ensures that
|
||||
// during the compile() phase, the module's metadata is available for selector scope
|
||||
// computation.
|
||||
this.scopeRegistry.registerNgModule(node, {declarations, imports, exports});
|
||||
this.scopeRegistry.registerNgModule(
|
||||
node, {declarations: declarationRefs, imports: importRefs, exports: exportRefs});
|
||||
|
||||
const valueContext = node.getSourceFile();
|
||||
|
||||
@ -131,13 +132,26 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
||||
typeContext = typeNode.getSourceFile();
|
||||
}
|
||||
|
||||
const bootstrap =
|
||||
bootstrapRefs.map(bootstrap => this._toR3Reference(bootstrap, valueContext, typeContext));
|
||||
const declarations =
|
||||
declarationRefs.map(decl => this._toR3Reference(decl, valueContext, typeContext));
|
||||
const imports = importRefs.map(imp => this._toR3Reference(imp, valueContext, typeContext));
|
||||
const exports = exportRefs.map(exp => this._toR3Reference(exp, valueContext, typeContext));
|
||||
|
||||
const isForwardReference = (ref: R3Reference) =>
|
||||
isExpressionForwardReference(ref.value, node.name !, valueContext);
|
||||
const containsForwardDecls = bootstrap.some(isForwardReference) ||
|
||||
declarations.some(isForwardReference) || imports.some(isForwardReference) ||
|
||||
exports.some(isForwardReference);
|
||||
|
||||
const ngModuleDef: R3NgModuleMetadata = {
|
||||
type: new WrappedNodeExpr(node.name !),
|
||||
bootstrap:
|
||||
bootstrap.map(bootstrap => this._toR3Reference(bootstrap, valueContext, typeContext)),
|
||||
declarations: declarations.map(decl => this._toR3Reference(decl, valueContext, typeContext)),
|
||||
exports: exports.map(exp => this._toR3Reference(exp, valueContext, typeContext)),
|
||||
imports: imports.map(imp => this._toR3Reference(imp, valueContext, typeContext)),
|
||||
bootstrap,
|
||||
declarations,
|
||||
exports,
|
||||
imports,
|
||||
containsForwardDecls,
|
||||
emitInline: false,
|
||||
// TODO: to be implemented as a part of FW-1004.
|
||||
schemas: [],
|
||||
@ -173,7 +187,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
|
||||
analysis: {
|
||||
ngModuleDef,
|
||||
ngInjectorDef,
|
||||
declarations,
|
||||
declarations: declarationRefs,
|
||||
metadataStmt: generateSetClassMetadataCall(
|
||||
node, this.reflector, this.defaultImportRecorder, this.isCore),
|
||||
},
|
||||
|
@ -278,3 +278,17 @@ export function combineResolvers(resolvers: ForeignFunctionResolver[]): ForeignF
|
||||
return null;
|
||||
};
|
||||
}
|
||||
|
||||
export function isExpressionForwardReference(
|
||||
expr: Expression, context: ts.Node, contextSource: ts.SourceFile): boolean {
|
||||
if (isWrappedTsNodeExpr(expr)) {
|
||||
const node = ts.getOriginalNode(expr.node);
|
||||
return node.getSourceFile() === contextSource && context.pos < node.pos;
|
||||
} else {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function isWrappedTsNodeExpr(expr: Expression): expr is WrappedNodeExpr<ts.Node> {
|
||||
return expr instanceof WrappedNodeExpr;
|
||||
}
|
||||
|
@ -643,6 +643,86 @@ describe('ngtsc behavioral tests', () => {
|
||||
expect(dtsContents).toContain('as i1 from "foo";');
|
||||
});
|
||||
|
||||
it('should compile NgModules with references to forward declared bootstrap components', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
import {Component, forwardRef, NgModule} from '@angular/core';
|
||||
|
||||
@NgModule({
|
||||
bootstrap: [forwardRef(() => Foo)],
|
||||
})
|
||||
export class FooModule {}
|
||||
|
||||
@Component({selector: 'foo', template: 'foo'})
|
||||
export class Foo {}
|
||||
`);
|
||||
|
||||
env.driveMain();
|
||||
|
||||
const jsContents = env.getContents('test.js');
|
||||
expect(jsContents).toContain('bootstrap: function () { return [Foo]; }');
|
||||
});
|
||||
|
||||
it('should compile NgModules with references to forward declared directives', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
import {Directive, forwardRef, NgModule} from '@angular/core';
|
||||
|
||||
@NgModule({
|
||||
declarations: [forwardRef(() => Foo)],
|
||||
})
|
||||
export class FooModule {}
|
||||
|
||||
@Directive({selector: 'foo'})
|
||||
export class Foo {}
|
||||
`);
|
||||
|
||||
env.driveMain();
|
||||
|
||||
const jsContents = env.getContents('test.js');
|
||||
expect(jsContents).toContain('declarations: function () { return [Foo]; }');
|
||||
});
|
||||
|
||||
it('should compile NgModules with references to forward declared imports', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
import {forwardRef, NgModule} from '@angular/core';
|
||||
|
||||
@NgModule({
|
||||
imports: [forwardRef(() => BarModule)],
|
||||
})
|
||||
export class FooModule {}
|
||||
|
||||
@NgModule({})
|
||||
export class BarModule {}
|
||||
`);
|
||||
|
||||
env.driveMain();
|
||||
|
||||
const jsContents = env.getContents('test.js');
|
||||
expect(jsContents).toContain('imports: function () { return [BarModule]; }');
|
||||
});
|
||||
|
||||
it('should compile NgModules with references to forward declared exports', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
import {forwardRef, NgModule} from '@angular/core';
|
||||
|
||||
@NgModule({
|
||||
exports: [forwardRef(() => BarModule)],
|
||||
})
|
||||
export class FooModule {}
|
||||
|
||||
@NgModule({})
|
||||
export class BarModule {}
|
||||
`);
|
||||
|
||||
env.driveMain();
|
||||
|
||||
const jsContents = env.getContents('test.js');
|
||||
expect(jsContents).toContain('exports: function () { return [BarModule]; }');
|
||||
});
|
||||
|
||||
it('should compile Pipes without errors', () => {
|
||||
env.tsconfig();
|
||||
env.write('test.ts', `
|
||||
|
Reference in New Issue
Block a user