From 4341a5f4cb7969b5c06d46a6afbd95cbccab8d95 Mon Sep 17 00:00:00 2001 From: nickreid Date: Mon, 2 Aug 2021 14:11:58 -0700 Subject: [PATCH] fix(compiler-cli): Emit type annotations for synthesized decorator fields (#43021) Previously, the decorator transformer was annotating the synthesized properties with TS type annotations. However, because it ran after the JSDoc transformer, the TS types were just dropped from the emitted JS. Attempting to move the decorator transformer before the JSDoc transformer causes tsickle crashes because synthetic AST fragments are not attached to a SourceFile node. PR Close #43021 --- .../downlevel_decorators_transform.ts | 196 ++++++++---------- packages/compiler-cli/test/ngc_spec.ts | 2 +- .../downlevel_decorators_transform_spec.ts | 11 +- 3 files changed, 96 insertions(+), 113 deletions(-) diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts index a5533f38a10..f9da5fa2f3e 100644 --- a/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts +++ b/packages/compiler-cli/src/transformers/downlevel_decorators_transform.ts @@ -40,20 +40,7 @@ function isAngularDecorator(decorator: Decorator, isCore: boolean): boolean { ##################################################################### */ -/** - * Creates the AST for the decorator field type annotation, which has the form - * { type: Function, args?: any[] }[] - */ -function createDecoratorInvocationType(): ts.TypeNode { - const typeElements: ts.TypeElement[] = []; - typeElements.push(ts.createPropertySignature( - undefined, 'type', undefined, - ts.createTypeReferenceNode(ts.createIdentifier('Function'), undefined), undefined)); - typeElements.push(ts.createPropertySignature( - undefined, 'args', ts.createToken(ts.SyntaxKind.QuestionToken), - ts.createArrayTypeNode(ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)), undefined)); - return ts.createArrayTypeNode(ts.createTypeLiteralNode(typeElements)); -} +const DECORATOR_INVOCATION_JSDOC_TYPE = '!Array<{type: !Function, args: (undefined|!Array)}>'; /** * Extracts the type of the decorator (the function or expression invoked), as well as all the @@ -100,68 +87,6 @@ function extractMetadataFromSingleDecorator( return ts.createObjectLiteral(metadataProperties); } -/** - * Takes a list of decorator metadata object ASTs and produces an AST for a - * static class property of an array of those metadata objects. - */ -function createDecoratorClassProperty(decoratorList: ts.ObjectLiteralExpression[]) { - const modifier = ts.createToken(ts.SyntaxKind.StaticKeyword); - const type = createDecoratorInvocationType(); - const initializer = ts.createArrayLiteral(decoratorList, true); - // NB: the .decorators property does not get a @nocollapse property. There is - // no good reason why - it means .decorators is not runtime accessible if you - // compile with collapse properties, whereas propDecorators is, which doesn't - // follow any stringent logic. However this has been the case previously, and - // adding it back in leads to substantial code size increases as Closure fails - // to tree shake these props without @nocollapse. - return ts.createProperty(undefined, [modifier], 'decorators', undefined, type, initializer); -} - -/** - * Creates the AST for the 'ctorParameters' field type annotation: - * () => ({ type: any, decorators?: {type: Function, args?: any[]}[] }|null)[] - */ -function createCtorParametersClassPropertyType(): ts.TypeNode { - // Sorry about this. Try reading just the string literals below. - const typeElements: ts.TypeElement[] = []; - typeElements.push(ts.createPropertySignature( - undefined, 'type', undefined, - ts.createTypeReferenceNode(ts.createIdentifier('any'), undefined), undefined)); - typeElements.push(ts.createPropertySignature( - undefined, 'decorators', ts.createToken(ts.SyntaxKind.QuestionToken), - ts.createArrayTypeNode(ts.createTypeLiteralNode([ - ts.createPropertySignature( - undefined, 'type', undefined, - ts.createTypeReferenceNode(ts.createIdentifier('Function'), undefined), undefined), - ts.createPropertySignature( - undefined, 'args', ts.createToken(ts.SyntaxKind.QuestionToken), - ts.createArrayTypeNode( - ts.createTypeReferenceNode(ts.createIdentifier('any'), undefined)), - undefined), - ])), - undefined)); - - return ts.createFunctionTypeNode(undefined, [], ts.createArrayTypeNode(ts.createUnionTypeNode([ - ts.createTypeLiteralNode(typeElements), - ts.createLiteralTypeNode(ts.createNull()), - ]))); -} - -/** - * Sets a Closure \@nocollapse synthetic comment on the given node. This prevents Closure Compiler - * from collapsing the apparently static property, which would make it impossible to find for code - * trying to detect it at runtime. - */ -function addNoCollapseComment(n: ts.Node) { - ts.setSyntheticLeadingComments(n, [{ - kind: ts.SyntaxKind.MultiLineCommentTrivia, - text: '* @nocollapse ', - pos: -1, - end: -1, - hasTrailingNewLine: true - }]); -} - /** * createCtorParametersClassProperty creates a static 'ctorParameters' property containing * downleveled decorator information. @@ -207,46 +132,31 @@ function createCtorParametersClassProperty( const initializer = ts.createArrowFunction( undefined, undefined, [], undefined, ts.createToken(ts.SyntaxKind.EqualsGreaterThanToken), ts.createArrayLiteral(params, true)); - const type = createCtorParametersClassPropertyType(); const ctorProp = ts.createProperty( - undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], 'ctorParameters', undefined, type, - initializer); + undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], 'ctorParameters', undefined, + undefined, initializer); if (isClosureCompilerEnabled) { - addNoCollapseComment(ctorProp); + ts.setSyntheticLeadingComments(ctorProp, [ + { + kind: ts.SyntaxKind.MultiLineCommentTrivia, + text: [ + `*`, + ` * @type {function(): !Array<(null|{`, + ` * type: ?,`, + ` * decorators: (undefined|${DECORATOR_INVOCATION_JSDOC_TYPE}),`, + ` * })>}`, + ` * @nocollapse`, + ` `, + ].join('\n'), + pos: -1, + end: -1, + hasTrailingNewLine: true, + }, + ]); } return ctorProp; } -/** - * createPropDecoratorsClassProperty creates a static 'propDecorators' property containing type - * information for every property that has a decorator applied. - * - * static propDecorators: {[key: string]: {type: Function, args?: any[]}[]} = { - * propA: [{type: MyDecorator, args: [1, 2]}, ...], - * ... - * }; - */ -function createPropDecoratorsClassProperty( - diagnostics: ts.Diagnostic[], properties: Map): ts.PropertyDeclaration { - // `static propDecorators: {[key: string]: ` + {type: Function, args?: any[]}[] + `} = {\n`); - const entries: ts.ObjectLiteralElementLike[] = []; - for (const [name, decorators] of properties.entries()) { - entries.push(ts.createPropertyAssignment( - name, - ts.createArrayLiteral( - decorators.map(deco => extractMetadataFromSingleDecorator(deco, diagnostics))))); - } - const initializer = ts.createObjectLiteral(entries, true); - const type = ts.createTypeLiteralNode([ts.createIndexSignature( - undefined, undefined, [ts.createParameter( - undefined, undefined, undefined, 'key', undefined, - ts.createTypeReferenceNode('string', undefined), undefined)], - createDecoratorInvocationType())]); - return ts.createProperty( - undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], 'propDecorators', undefined, type, - initializer); -} - /** * Returns an expression representing the (potentially) value part for the given node. * @@ -346,6 +256,72 @@ export function getDownlevelDecoratorsTransform( typeChecker: ts.TypeChecker, host: ReflectionHost, diagnostics: ts.Diagnostic[], isCore: boolean, isClosureCompilerEnabled: boolean, skipClassDecorators: boolean): ts.TransformerFactory { + function addJSDocTypeAnnotation(node: ts.Node, jsdocType: string): void { + if (!isClosureCompilerEnabled) { + return; + } + + ts.setSyntheticLeadingComments(node, [ + { + kind: ts.SyntaxKind.MultiLineCommentTrivia, + text: `* @type {${jsdocType}} `, + pos: -1, + end: -1, + hasTrailingNewLine: true, + }, + ]); + } + + /** + * Takes a list of decorator metadata object ASTs and produces an AST for a + * static class property of an array of those metadata objects. + */ + function createDecoratorClassProperty(decoratorList: ts.ObjectLiteralExpression[]) { + const modifier = ts.createToken(ts.SyntaxKind.StaticKeyword); + const initializer = ts.createArrayLiteral(decoratorList, true); + // NB: the .decorators property does not get a @nocollapse property. There + // is no good reason why - it means .decorators is not runtime accessible + // if you compile with collapse properties, whereas propDecorators is, + // which doesn't follow any stringent logic. However this has been the + // case previously, and adding it back in leads to substantial code size + // increases as Closure fails to tree shake these props + // without @nocollapse. + const prop = + ts.createProperty(undefined, [modifier], 'decorators', undefined, undefined, initializer); + addJSDocTypeAnnotation(prop, DECORATOR_INVOCATION_JSDOC_TYPE); + return prop; + } + + /** + * createPropDecoratorsClassProperty creates a static 'propDecorators' + * property containing type information for every property that has a + * decorator applied. + * + * static propDecorators: {[key: string]: {type: Function, args?: + * any[]}[]} = { propA: [{type: MyDecorator, args: [1, 2]}, ...], + * ... + * }; + */ + function createPropDecoratorsClassProperty( + diagnostics: ts.Diagnostic[], + properties: Map): ts.PropertyDeclaration { + // `static propDecorators: {[key: string]: ` + {type: Function, args?: + // any[]}[] + `} = {\n`); + const entries: ts.ObjectLiteralElementLike[] = []; + for (const [name, decorators] of properties.entries()) { + entries.push(ts.createPropertyAssignment( + name, + ts.createArrayLiteral( + decorators.map(deco => extractMetadataFromSingleDecorator(deco, diagnostics))))); + } + const initializer = ts.createObjectLiteral(entries, true); + const prop = ts.createProperty( + undefined, [ts.createToken(ts.SyntaxKind.StaticKeyword)], 'propDecorators', undefined, + undefined, initializer); + addJSDocTypeAnnotation(prop, `!Object`); + return prop; + } + return (context: ts.TransformationContext) => { // Ensure that referenced type symbols are not elided by TypeScript. Imports for // such parameter type symbols previously could be type-only, but now might be also diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index ed8b9a9f154..d7c7a0e8873 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -715,7 +715,7 @@ describe('ngc transformer command-line', () => { const mymoduleSource = fs.readFileSync(mymodulejs, 'utf8'); expect(mymoduleSource).toContain('@fileoverview added by tsickle'); expect(mymoduleSource).toContain('@param {?} p'); - expect(mymoduleSource).toMatch(/\/\*\* @nocollapse \*\/\s+MyComp\.ctorParameters = /); + expect(mymoduleSource).toContain('@nocollapse'); }); }); diff --git a/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts index 7aa942901c9..eaee4b84da1 100644 --- a/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts +++ b/packages/compiler-cli/test/transformers/downlevel_decorators_transform_spec.ts @@ -424,11 +424,18 @@ describe('downlevel decorator transform', () => { expect(diagnostics.length).toBe(0); expect(output).toContain(dedent` + /** @type {!Array<{type: !Function, args: (undefined|!Array)}>} */ MyDir.decorators = [ { type: core_1.Directive } ]; - /** @nocollapse */ - MyDir.ctorParameters = () => [ + /** + * @type {function(): !Array<(null|{ + * type: ?, + * decorators: (undefined|!Array<{type: !Function, args: (undefined|!Array)}>), + * })>} + * @nocollapse + */ + MyDir.ctorParameters = () => [ { type: ClassInject } ]; `);