fix(ivy): ngtsc should pay attention to declaration order (#25392)
When generating the 'directives:' property of ngComponentDef, ngtsc needs to be conscious of declaration order. If a directive being written into the array is declarated after the component currently being compiled, then the entire directives array needs to be wrapped in a closure. This commit fixes ngtsc to pay attention to such ordering issues within directives arrays. PR Close #25392
This commit is contained in:
parent
6f085f8610
commit
2befc65777
|
@ -142,6 +142,7 @@ export class ComponentDecoratorHandler implements DecoratorHandler<R3ComponentMe
|
|||
// analyzed and the full compilation scope for the component can be realized.
|
||||
pipes: EMPTY_MAP,
|
||||
directives: EMPTY_MAP,
|
||||
wrapDirectivesInClosure: false,
|
||||
}
|
||||
};
|
||||
}
|
||||
|
@ -155,7 +156,9 @@ export class ComponentDecoratorHandler implements DecoratorHandler<R3ComponentMe
|
|||
// Replace the empty components and directives from the analyze() step with a fully expanded
|
||||
// scope. This is possible now because during compile() the whole compilation unit has been
|
||||
// fully analyzed.
|
||||
analysis = {...analysis, ...scope};
|
||||
const {directives, pipes, containsForwardDecls} = scope;
|
||||
const wrapDirectivesInClosure: boolean = !!containsForwardDecls;
|
||||
analysis = {...analysis, directives, pipes, wrapDirectivesInClosure};
|
||||
}
|
||||
|
||||
const res = compileComponentFromMetadata(analysis, pool, makeBindingParser());
|
||||
|
|
|
@ -6,7 +6,7 @@
|
|||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
|
||||
import {Expression, ExternalExpr, ExternalReference} from '@angular/compiler';
|
||||
import {Expression, ExternalExpr, ExternalReference, WrappedNodeExpr} from '@angular/compiler';
|
||||
import * as ts from 'typescript';
|
||||
|
||||
import {ReflectionHost} from '../../host';
|
||||
|
@ -33,6 +33,7 @@ export interface ModuleData {
|
|||
export interface CompilationScope<T> {
|
||||
directives: Map<string, T>;
|
||||
pipes: Map<string, T>;
|
||||
containsForwardDecls?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -146,7 +147,7 @@ export class SelectorScopeRegistry {
|
|||
|
||||
// The scope as cached is in terms of References, not Expressions. Converting between them
|
||||
// requires knowledge of the context file (in this case, the component node's source file).
|
||||
return convertScopeToExpressions(scope, node.getSourceFile());
|
||||
return convertScopeToExpressions(scope, node);
|
||||
}
|
||||
|
||||
// This is the first time the scope for this module is being computed.
|
||||
|
@ -179,7 +180,7 @@ export class SelectorScopeRegistry {
|
|||
this._compilationScopeCache.set(node, scope);
|
||||
|
||||
// Convert References to Expressions in the context of the component's source file.
|
||||
return convertScopeToExpressions(scope, node.getSourceFile());
|
||||
return convertScopeToExpressions(scope, node);
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -390,8 +391,40 @@ function convertReferenceMap(
|
|||
}
|
||||
|
||||
function convertScopeToExpressions(
|
||||
scope: CompilationScope<Reference>, context: ts.SourceFile): CompilationScope<Expression> {
|
||||
const directives = convertReferenceMap(scope.directives, context);
|
||||
const pipes = convertReferenceMap(scope.pipes, context);
|
||||
return {directives, pipes};
|
||||
scope: CompilationScope<Reference>, context: ts.Declaration): CompilationScope<Expression> {
|
||||
const sourceContext = ts.getOriginalNode(context).getSourceFile();
|
||||
const directives = convertReferenceMap(scope.directives, sourceContext);
|
||||
const pipes = convertReferenceMap(scope.pipes, sourceContext);
|
||||
const declPointer = maybeUnwrapNameOfDeclaration(context);
|
||||
let containsForwardDecls = false;
|
||||
directives.forEach(expr => {
|
||||
containsForwardDecls =
|
||||
containsForwardDecls || isExpressionForwardReference(expr, declPointer, sourceContext);
|
||||
});
|
||||
!containsForwardDecls && pipes.forEach(expr => {
|
||||
containsForwardDecls =
|
||||
containsForwardDecls || isExpressionForwardReference(expr, declPointer, sourceContext);
|
||||
});
|
||||
return {directives, pipes, containsForwardDecls};
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
function isWrappedTsNodeExpr(expr: Expression): expr is WrappedNodeExpr<ts.Node> {
|
||||
return expr instanceof WrappedNodeExpr;
|
||||
}
|
||||
|
||||
function maybeUnwrapNameOfDeclaration(decl: ts.Declaration): ts.Declaration|ts.Identifier {
|
||||
if ((ts.isClassDeclaration(decl) || ts.isVariableDeclaration(decl)) && decl.name !== undefined &&
|
||||
ts.isIdentifier(decl.name)) {
|
||||
return decl.name;
|
||||
}
|
||||
return decl;
|
||||
}
|
|
@ -1002,7 +1002,7 @@ describe('compiler compliance', () => {
|
|||
$r3$.ɵEe(1, "div", $e0_attrs$);
|
||||
}
|
||||
},
|
||||
directives:[SomeDirective]
|
||||
directives: function () { return [SomeDirective]; }
|
||||
});`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
|
@ -1582,7 +1582,7 @@ describe('compiler compliance', () => {
|
|||
}
|
||||
if (rf & 2) { $r3$.ɵp(1,"forOf",$r3$.ɵb(ctx.items)); }
|
||||
},
|
||||
directives: [ForOfDirective]
|
||||
directives: function() { return [ForOfDirective]; }
|
||||
});
|
||||
`;
|
||||
|
||||
|
@ -1660,7 +1660,7 @@ describe('compiler compliance', () => {
|
|||
$r3$.ɵp(1, "forOf", $r3$.ɵb(ctx.items));
|
||||
}
|
||||
},
|
||||
directives: [ForOfDirective]
|
||||
directives: function() { return [ForOfDirective]; }
|
||||
});
|
||||
`;
|
||||
|
||||
|
@ -1758,7 +1758,7 @@ describe('compiler compliance', () => {
|
|||
$r3$.ɵp(1, "forOf", $r3$.ɵb(ctx.items));
|
||||
}
|
||||
},
|
||||
directives: [ForOfDirective]
|
||||
directives: function () { return [ForOfDirective]; }
|
||||
});`;
|
||||
|
||||
const result = compile(files, angularFiles);
|
||||
|
|
|
@ -715,4 +715,36 @@ describe('ngtsc behavioral tests', () => {
|
|||
expect(jsContents)
|
||||
.toContain('function GrandChild_Factory(t) { return new (t || GrandChild)(); }');
|
||||
});
|
||||
|
||||
it('should wrap "directives" in component metadata in a closure when forward references are present',
|
||||
() => {
|
||||
writeConfig();
|
||||
write('test.ts', `
|
||||
import {Component, NgModule} from '@angular/core';
|
||||
|
||||
@Component({
|
||||
selector: 'cmp-a',
|
||||
template: '<cmp-b></cmp-b>',
|
||||
})
|
||||
class CmpA {}
|
||||
|
||||
@Component({
|
||||
selector: 'cmp-b',
|
||||
template: 'This is B',
|
||||
})
|
||||
class CmpB {}
|
||||
|
||||
@NgModule({
|
||||
declarations: [CmpA, CmpB],
|
||||
})
|
||||
class Module {}
|
||||
`);
|
||||
|
||||
const exitCode = main(['-p', basePath], errorSpy);
|
||||
expect(errorSpy).not.toHaveBeenCalled();
|
||||
expect(exitCode).toBe(0);
|
||||
|
||||
const jsContents = getContents('test.js');
|
||||
expect(jsContents).toContain('directives: function () { return [CmpB]; }');
|
||||
});
|
||||
});
|
||||
|
|
|
@ -144,6 +144,13 @@ export interface R3ComponentMetadata extends R3DirectiveMetadata {
|
|||
* scope of the compilation.
|
||||
*/
|
||||
directives: Map<string, o.Expression>;
|
||||
|
||||
/**
|
||||
* Whether to wrap the 'directives' array, if one is generated, in a closure.
|
||||
*
|
||||
* This is done when the directives contain forward references.
|
||||
*/
|
||||
wrapDirectivesInClosure: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -165,7 +165,11 @@ export function compileComponentFromMetadata(
|
|||
|
||||
// e.g. `directives: [MyDirective]`
|
||||
if (directivesUsed.size) {
|
||||
definitionMap.set('directives', o.literalArr(Array.from(directivesUsed)));
|
||||
let directivesExpr: o.Expression = o.literalArr(Array.from(directivesUsed));
|
||||
if (meta.wrapDirectivesInClosure) {
|
||||
directivesExpr = o.fn([], [new o.ReturnStatement(directivesExpr)]);
|
||||
}
|
||||
definitionMap.set('directives', directivesExpr);
|
||||
}
|
||||
|
||||
// e.g. `pipes: [MyPipe]`
|
||||
|
@ -241,6 +245,7 @@ export function compileComponentFromRender2(
|
|||
directives: typeMapToExpressionMap(directiveTypeBySel, outputCtx),
|
||||
pipes: typeMapToExpressionMap(pipeTypeByName, outputCtx),
|
||||
viewQueries: queriesFromGlobalMetadata(component.viewQueries, outputCtx),
|
||||
wrapDirectivesInClosure: false,
|
||||
};
|
||||
const res = compileComponentFromMetadata(meta, outputCtx.constantPool, bindingParser);
|
||||
|
||||
|
|
|
@ -73,6 +73,7 @@ export function compileComponent(type: Type<any>, metadata: Component): void {
|
|||
directives: new Map(),
|
||||
pipes: new Map(),
|
||||
viewQueries: [],
|
||||
wrapDirectivesInClosure: false,
|
||||
},
|
||||
constantPool, makeBindingParser());
|
||||
const preStatements = [...constantPool.statements, ...res.statements];
|
||||
|
|
Loading…
Reference in New Issue