From 3858b26211f55dbbae9be819a64a475d8291c217 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 20 Oct 2019 22:45:28 +0200 Subject: [PATCH] refactor(ivy): mark synthetic decorators explicitly (#33362) In ngcc's migration system, synthetic decorators can be injected into a compilation to ensure that certain classes are compiled with Angular logic, where the original library code did not include the necessary decorators. Prior to this change, synthesized decorators would have a fake AST structure as associated node and a made-up identifier. In theory, this may introduce issues downstream: 1) a decorator's node is used for diagnostics, so it must have position information. Having fake AST nodes without a position is therefore a problem. Note that this is currently not a problem in practice, as injected synthesized decorators would not produce any diagnostics. 2) the decorator's identifier should refer to an imported symbol. Therefore, it is required that the symbol is actually imported. Moreover, bundle formats such as UMD and CommonJS use namespaces for imports, so a bare `ts.Identifier` would not be suitable to use as identifier. This was also not a problem in practice, as the identifier is only used in the `setClassMetadata` generated code, which is omitted for synthetically injected decorators. To remedy these potential issues, this commit makes a decorator's identifier optional and switches its node over from a fake AST structure to the class' name. PR Close #33362 --- .../compiler-cli/ngcc/src/migrations/utils.ts | 48 ++++++++--------- .../ngcc/src/rendering/renderer.ts | 3 ++ .../host/commonjs_host_import_helper_spec.ts | 2 +- .../host/esm2015_host_import_helper_spec.ts | 8 +-- .../test/host/esm5_host_import_helper_spec.ts | 8 +-- .../test/host/umd_host_import_helper_spec.ts | 2 +- .../commonjs_rendering_formatter_spec.ts | 12 ++--- .../esm5_rendering_formatter_spec.ts | 12 ++--- .../rendering/esm_rendering_formatter_spec.ts | 14 ++--- .../rendering/umd_rendering_formatter_spec.ts | 12 ++--- .../src/ngtsc/annotations/src/component.ts | 5 +- .../src/ngtsc/annotations/src/directive.ts | 6 +-- .../src/ngtsc/annotations/src/injectable.ts | 6 ++- .../src/ngtsc/annotations/src/metadata.ts | 3 ++ .../src/ngtsc/annotations/src/ng_module.ts | 2 +- .../src/ngtsc/annotations/src/pipe.ts | 6 ++- .../src/ngtsc/annotations/src/util.ts | 7 +-- .../src/ngtsc/reflection/src/host.ts | 53 ++++++++++++++++--- 18 files changed, 131 insertions(+), 78 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/migrations/utils.ts b/packages/compiler-cli/ngcc/src/migrations/utils.ts index 219c2ab474..a305986de0 100644 --- a/packages/compiler-cli/ngcc/src/migrations/utils.ts +++ b/packages/compiler-cli/ngcc/src/migrations/utils.ts @@ -37,20 +37,14 @@ export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator { // TODO: At the moment ngtsc does not accept a directive with no selector ts.createPropertyAssignment('selector', ts.createStringLiteral('NGCC_DUMMY')), ]); - const decoratorType = ts.createIdentifier('Directive'); - const decoratorNode = ts.createObjectLiteral([ - ts.createPropertyAssignment('type', decoratorType), - ts.createPropertyAssignment('args', ts.createArrayLiteral([selectorArg])), - ]); - - setParentPointers(clazz.getSourceFile(), decoratorNode); return { name: 'Directive', - identifier: decoratorType, + identifier: null, import: {name: 'Directive', from: '@angular/core'}, - node: decoratorNode, - args: [selectorArg], + node: null, + synthesizedFor: clazz.name, + args: [reifySourceFile(selectorArg)], }; } @@ -58,27 +52,33 @@ export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator { * Create an empty `Injectable` decorator that will be associated with the `clazz`. */ export function createInjectableDecorator(clazz: ClassDeclaration): Decorator { - const decoratorType = ts.createIdentifier('Injectable'); - const decoratorNode = ts.createObjectLiteral([ - ts.createPropertyAssignment('type', decoratorType), - ts.createPropertyAssignment('args', ts.createArrayLiteral([])), - ]); - - setParentPointers(clazz.getSourceFile(), decoratorNode); - return { name: 'Injectable', - identifier: decoratorType, + identifier: null, import: {name: 'Injectable', from: '@angular/core'}, - node: decoratorNode, + node: null, + synthesizedFor: clazz.name, args: [], }; } +const EMPTY_SF = ts.createSourceFile('(empty)', '', ts.ScriptTarget.Latest); + /** - * Ensure that a tree of AST nodes have their parents wired up. + * Takes a `ts.Expression` and returns the same `ts.Expression`, but with an associated + * `ts.SourceFile`. + * + * This transformation is necessary to use synthetic `ts.Expression`s with the `PartialEvaluator`, + * and many decorator arguments are interpreted in this way. */ -export function setParentPointers(parent: ts.Node, child: ts.Node): void { - child.parent = parent; - ts.forEachChild(child, grandchild => setParentPointers(child, grandchild)); +function reifySourceFile(expr: ts.Expression): ts.Expression { + const printer = ts.createPrinter(); + const exprText = printer.printNode(ts.EmitHint.Unspecified, expr, EMPTY_SF); + const sf = ts.createSourceFile( + '(synthetic)', `const expr = ${exprText};`, ts.ScriptTarget.Latest, true, ts.ScriptKind.JS); + const stmt = sf.statements[0]; + if (!ts.isVariableStatement(stmt)) { + throw new Error(`Expected VariableStatement, got ${ts.SyntaxKind[stmt.kind]}`); + } + return stmt.declarationList.declarations[0].initializer !; } diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index 958ec67588..171c1ec27e 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -130,6 +130,9 @@ export class Renderer { } clazz.decorators.forEach(dec => { + if (dec.node === null) { + return; + } const decoratorArray = dec.node.parent !; if (!decoratorsToRemove.has(decoratorArray)) { decoratorsToRemove.set(decoratorArray, [dec.node]); diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts index 8163a75311..b9bce75af3 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_import_helper_spec.ts @@ -86,7 +86,7 @@ exports.OtherDirective = OtherDirective; const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('core.Directive'); + expect(decorator.identifier !.getText()).toEqual('core.Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts index 2b22192e88..3b617a2d9f 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts @@ -175,7 +175,7 @@ runInEachFileSystem(() => { const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -197,7 +197,7 @@ runInEachFileSystem(() => { const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -219,7 +219,7 @@ runInEachFileSystem(() => { const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: './directives'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -433,7 +433,7 @@ runInEachFileSystem(() => { const classNode = getDeclaration( program, _('/some_directive.js'), 'SomeDirective', isNamedVariableDeclaration); const classDecorators = host.getDecoratorsOfDeclaration(classNode) !; - const decoratorNode = classDecorators[0].node; + const decoratorNode = classDecorators[0].node !; const identifierOfDirective = ts.isCallExpression(decoratorNode) && ts.isIdentifier(decoratorNode.expression) ? decoratorNode.expression : diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts index b467be4e19..a2769b93e8 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -202,7 +202,7 @@ export { SomeDirective }; const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -224,7 +224,7 @@ export { SomeDirective }; const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -245,7 +245,7 @@ export { SomeDirective }; const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('Directive'); + expect(decorator.identifier !.getText()).toEqual('Directive'); expect(decorator.import).toEqual({name: 'Directive', from: './directives'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', @@ -470,7 +470,7 @@ export { SomeDirective }; const classNode = getDeclaration( program, _('/some_directive.js'), 'SomeDirective', isNamedVariableDeclaration); const classDecorators = host.getDecoratorsOfDeclaration(classNode) !; - const decoratorNode = classDecorators[0].node; + const decoratorNode = classDecorators[0].node !; const identifierOfDirective = ts.isCallExpression(decoratorNode) && ts.isIdentifier(decoratorNode.expression) ? diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts index 0896665b0c..1cd4ee7a27 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_import_helper_spec.ts @@ -79,7 +79,7 @@ runInEachFileSystem(() => { const decorator = decorators[0]; expect(decorator.name).toEqual('Directive'); - expect(decorator.identifier.getText()).toEqual('core.Directive'); + expect(decorator.identifier !.getText()).toEqual('core.Directive'); expect(decorator.import).toEqual({name: 'Directive', from: '@angular/core'}); expect(decorator.args !.map(arg => arg.getText())).toEqual([ '{ selector: \'[someDirective]\' }', diff --git a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts index 6564513056..14ddeeefdc 100644 --- a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts @@ -356,7 +356,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .not.toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`); @@ -377,7 +377,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`); @@ -398,7 +398,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT'); expect(output.toString()) @@ -423,7 +423,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -440,7 +440,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -458,7 +458,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); diff --git a/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts index 387b5197d4..1b7b64ee4d 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts @@ -345,7 +345,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .not.toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); @@ -364,7 +364,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); expect(output.toString()).toContain(`{ type: OtherA }`); @@ -383,7 +383,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT'); expect(output.toString()).toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); @@ -406,7 +406,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -423,7 +423,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -441,7 +441,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); diff --git a/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts index bda5e1da02..7dbb778cf0 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts @@ -255,7 +255,7 @@ A.decorators = [ decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .not.toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); @@ -276,7 +276,7 @@ A.decorators = [ decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); @@ -304,7 +304,7 @@ A.decorators = [ decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); // The decorator should have been removed correctly. expect(output.toString()).toContain('A.decorators = [ { type: OtherA }'); @@ -319,7 +319,7 @@ A.decorators = [ decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .toContain(`{ type: Directive, args: [{ selector: '[a]' }] },`); @@ -385,7 +385,7 @@ export { D }; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).not.toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -402,7 +402,7 @@ export { D }; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -420,7 +420,7 @@ export { D }; decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); diff --git a/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts index c67c5d5d7f..1a5158dd05 100644 --- a/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts @@ -425,7 +425,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .not.toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`); @@ -446,7 +446,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()) .toContain(`{ type: core.Directive, args: [{ selector: '[a]' }] },`); @@ -467,7 +467,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators ![0]; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); renderer.addDefinitions(output, compiledClass, 'SOME DEFINITION TEXT'); expect(output.toString()) @@ -490,7 +490,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'A') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).not.toContain(`core.Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -507,7 +507,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'B') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`core.Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); @@ -525,7 +525,7 @@ SOME DEFINITION TEXT decorationAnalyses.get(sourceFile) !.compiledClasses.find(c => c.name === 'C') !; const decorator = compiledClass.decorators !.find(d => d.name === 'Directive') !; const decoratorsToRemove = new Map(); - decoratorsToRemove.set(decorator.node.parent !, [decorator.node]); + decoratorsToRemove.set(decorator.node !.parent !, [decorator.node !]); renderer.removeDecorators(output, decoratorsToRemove); expect(output.toString()).toContain(`core.Directive({ selector: '[a]' }),`); expect(output.toString()).toContain(`OtherA()`); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 4578da8b48..23cbe87922 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -509,7 +509,7 @@ export class ComponentDecoratorHandler implements } if (decorator.args === null || decorator.args.length !== 1) { throw new FatalDiagnosticError( - ErrorCode.DECORATOR_ARITY_WRONG, decorator.node, + ErrorCode.DECORATOR_ARITY_WRONG, Decorator.nodeForError(decorator), `Incorrect number of arguments to @Component decorator`); } const meta = unwrapExpression(decorator.args[0]); @@ -624,7 +624,8 @@ export class ComponentDecoratorHandler implements } { if (!component.has('template')) { throw new FatalDiagnosticError( - ErrorCode.COMPONENT_MISSING_TEMPLATE, decorator.node, 'component is missing a template'); + ErrorCode.COMPONENT_MISSING_TEMPLATE, Decorator.nodeForError(decorator), + 'component is missing a template'); } const templateExpr = component.get('template') !; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index d4cd46cabd..47be27966c 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -121,7 +121,7 @@ export function extractDirectiveMetadata( directive = new Map(); } else if (decorator.args.length !== 1) { throw new FatalDiagnosticError( - ErrorCode.DECORATOR_ARITY_WRONG, decorator.node, + ErrorCode.DECORATOR_ARITY_WRONG, Decorator.nodeForError(decorator), `Incorrect number of arguments to @${decorator.name} decorator`); } else { const meta = unwrapExpression(decorator.args[0]); @@ -447,7 +447,7 @@ export function queriesFromFields( evaluator: PartialEvaluator): R3QueryMetadata[] { return fields.map(({member, decorators}) => { const decorator = decorators[0]; - const node = member.node || decorator.node; + const node = member.node || Decorator.nodeForError(decorator); // Throw in case of `@Input() @ContentChild('foo') foo: any`, which is not supported in Ivy if (member.decorators !.some(v => v.name === 'Input')) { @@ -465,7 +465,7 @@ export function queriesFromFields( 'Query decorator must go on a property-type member'); } return extractQueryMetadata( - decorator.node, decorator.name, decorator.args || [], member.name, reflector, evaluator); + node, decorator.name, decorator.args || [], member.name, reflector, evaluator); }); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index b2b6a4d583..c1ca25603b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -116,7 +116,8 @@ function extractInjectableMetadata( const typeArgumentCount = reflector.getGenericArityOfClass(clazz) || 0; if (decorator.args === null) { throw new FatalDiagnosticError( - ErrorCode.DECORATOR_NOT_CALLED, decorator.node, '@Injectable must be called'); + ErrorCode.DECORATOR_NOT_CALLED, Decorator.nodeForError(decorator), + '@Injectable must be called'); } if (decorator.args.length === 0) { return { @@ -202,7 +203,8 @@ function extractInjectableCtorDeps( strictCtorDeps: boolean) { if (decorator.args === null) { throw new FatalDiagnosticError( - ErrorCode.DECORATOR_NOT_CALLED, decorator.node, '@Injectable must be called'); + ErrorCode.DECORATOR_NOT_CALLED, Decorator.nodeForError(decorator), + '@Injectable must be called'); } let ctorDeps: R3DependencyMetadata[]|'invalid'|null = null; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts index 595a9957cf..4c82fec506 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts @@ -122,6 +122,9 @@ function classMemberToMetadata( * Convert a reflected decorator to metadata. */ function decoratorToMetadata(decorator: Decorator): ts.ObjectLiteralExpression { + if (decorator.identifier === null) { + throw new Error('Illegal state: synthesized decorator cannot be emitted in class metadata.'); + } // Decorators have a type. const properties: ts.ObjectLiteralElementLike[] = [ ts.createPropertyAssignment('type', ts.getMutableClone(decorator.identifier)), diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index c40d354581..e8e3f48af0 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -67,7 +67,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler 1) { throw new FatalDiagnosticError( - ErrorCode.DECORATOR_ARITY_WRONG, decorator.node, + ErrorCode.DECORATOR_ARITY_WRONG, Decorator.nodeForError(decorator), `Incorrect number of arguments to @NgModule decorator`); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index d8668b439a..0857548a99 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -53,11 +53,13 @@ export class PipeDecoratorHandler implements DecoratorHandler { + if (decorator.node !== null) { + return decorator.node; + } else { + // TODO(alxhub): we can't rely on narrowing until TS 3.6 is in g3. + return (decorator as SyntheticDecorator).synthesizedFor; + } + }, +}; + /** * A decorator is identified by either a simple identifier (e.g. `Decorator`) or, in some cases, * a namespaced property access (e.g. `core.Decorator`).