From 5ebc800fd6b78f24c462fce4db4c8e2c41c6cdb1 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Tue, 1 Oct 2024 12:01:26 -0600 Subject: [PATCH 01/11] WIP: Figure out decorators --- demo/decorators.ts | 21 +++++++++++++++++++ demo/tsconfig.json | 1 + demo/using_decorators.ts | 28 ++++++++++++++++++++++++++ src/decorator_downlevel_transformer.ts | 13 ++++++++++++ tsconfig.json | 3 ++- 5 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 demo/decorators.ts create mode 100644 demo/using_decorators.ts diff --git a/demo/decorators.ts b/demo/decorators.ts new file mode 100644 index 000000000..b91a140e1 --- /dev/null +++ b/demo/decorators.ts @@ -0,0 +1,21 @@ +export function classDecorator(constructor: Function) { + Object.seal(constructor); + Object.seal(constructor.prototype); +} + +export function methodDecorator(value: boolean) { + return function (target: any, propertyKey: string, descriptor?: PropertyDescriptor) { + if (descriptor) { + descriptor.enumerable = value; + } + }; +} + +export function accessorDecorator(value: boolean,) { + return function (target: any, propertyKey: string, descriptor?: PropertyDescriptor) { + if (descriptor) { + descriptor.configurable = value; + } + }; +} + diff --git a/demo/tsconfig.json b/demo/tsconfig.json index 60753679f..15b499228 100644 --- a/demo/tsconfig.json +++ b/demo/tsconfig.json @@ -7,6 +7,7 @@ "strict": true, "moduleResolution": "Bundler", "esModuleInterop": true, + "experimentalDecorators": true, }, "exclude": [ "test.ts" diff --git a/demo/using_decorators.ts b/demo/using_decorators.ts new file mode 100644 index 000000000..0987429ad --- /dev/null +++ b/demo/using_decorators.ts @@ -0,0 +1,28 @@ +import { accessorDecorator, classDecorator, methodDecorator } from "./decorators"; + +@classDecorator +export class Person { + private name_: string; + constructor(name: string) { + this.name_ = name; + } + + @accessorDecorator(true) + get name() { + return this.name_; + } + + @methodDecorator(true) + greet() { + console.log(`Hello, my name is ${this.name}.`); + } + } + + const p = new Person("Ron"); + p.greet(); + + + export class Employee { + constructor(private age_: number) {} + get age() { return this.age_; } + } \ No newline at end of file diff --git a/src/decorator_downlevel_transformer.ts b/src/decorator_downlevel_transformer.ts index 1b2f6f546..37743969e 100644 --- a/src/decorator_downlevel_transformer.ts +++ b/src/decorator_downlevel_transformer.ts @@ -530,6 +530,19 @@ export function decoratorDownlevelTransformer( if (decoratedProperties.size) { newMembers.push(createPropDecoratorsClassProperty(diagnostics, decoratedProperties)); } + /** + * I'm pretty sure this is where the class ends up turning from + * + * @classDecorator + * class Person { ... } + * + * into + * + * let Person = class Person { ... } + * Person = __decorate([ + * classDecorator + * ], Person); + */ return ts.factory.updateClassDeclaration( classDecl, modifiersToKeep.length ? modifiersToKeep : undefined, classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, diff --git a/tsconfig.json b/tsconfig.json index 2b9334885..ab0b76c24 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -18,7 +18,8 @@ "strict": true, "outDir": "out", "paths": {"tsickle": ["./src/tsickle.ts"]}, - "esModuleInterop": true + "esModuleInterop": true, + "experimentalDecorators": true }, "include": [ "src/*.ts", From 93b553d35253cf12230129f8d55df84194174850 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Thu, 3 Oct 2024 15:43:06 -0600 Subject: [PATCH 02/11] WIP: handle processing decorators even with googmodules is false --- src/decorators.ts | 19 +++++++++++++++---- src/tsickle.ts | 5 +++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/decorators.ts b/src/decorators.ts index fb8523ad1..c073603bd 100644 --- a/src/decorators.ts +++ b/src/decorators.ts @@ -94,7 +94,7 @@ function isExportingDecorator(decorator: ts.Decorator, typeChecker: ts.TypeCheck * ], Foo.prototype, * __googReflect.objectProperty("prop", Foo.prototype), void 0); */ -export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: ts.Diagnostic[]) { +export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: ts.Diagnostic[], useGoogModule = true) { return (context: ts.TransformationContext) => { const result: ts.Transformer = (sourceFile: ts.SourceFile) => { let nodeNeedingGoogReflect: undefined|ts.Node = undefined; @@ -113,13 +113,13 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: if (nodeNeedingGoogReflect !== undefined) { const statements = [...updatedSourceFile.statements]; const googModuleIndex = statements.findIndex(isGoogModuleStatement); - if (googModuleIndex === -1) { + if (useGoogModule && googModuleIndex === -1) { reportDiagnostic( diagnostics, nodeNeedingGoogReflect, 'Internal tsickle error: could not find goog.module statement to import __tsickle_googReflect for decorator compilation.'); return sourceFile; } - const googRequireReflectObjectProperty = + const googRequireReflectObjectProperty = useGoogModule ? ts.factory.createVariableStatement( undefined, ts.factory.createVariableDeclarationList( @@ -131,7 +131,18 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: ts.factory.createIdentifier('goog'), 'require'), undefined, [ts.factory.createStringLiteral('goog.reflect')]))], - ts.NodeFlags.Const)); + ts.NodeFlags.Const)) : + ts.factory.createImportDeclaration( + undefined, + ts.factory.createImportClause( + false, + ts.factory.createIdentifier('__tsickle_googReflect'), + undefined + ), + ts.factory.createStringLiteral('goog:goog.reflect') + ); + + // The boilerplate we produce has a goog.module line, then two related // lines dealing with the `module` variable. Insert our goog.require // after that to avoid visually breaking up the module info, and to be diff --git a/src/tsickle.ts b/src/tsickle.ts index 3b10f073a..0856614ea 100644 --- a/src/tsickle.ts +++ b/src/tsickle.ts @@ -248,6 +248,11 @@ export function emit( transformDecoratorsOutputForClosurePropertyRenaming( tsickleDiagnostics)); tsTransformers.after!.push(transformDecoratorJsdoc()); + } else if (host.transformTypesToClosure) { + tsTransformers.after!.push( + transformDecoratorsOutputForClosurePropertyRenaming( + tsickleDiagnostics, false)); + tsTransformers.after!.push(transformDecoratorJsdoc()); } if (host.addDtsClutzAliases) { tsTransformers.afterDeclarations!.push( From f42c919c16499c54870fd774385f460991b49db3 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Mon, 7 Oct 2024 13:59:07 -0600 Subject: [PATCH 03/11] Use JSCompiler_rename --- src/decorator_downlevel_transformer.ts | 13 ----- src/decorators.ts | 76 ++++++++++++++------------ 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/src/decorator_downlevel_transformer.ts b/src/decorator_downlevel_transformer.ts index 37743969e..1b2f6f546 100644 --- a/src/decorator_downlevel_transformer.ts +++ b/src/decorator_downlevel_transformer.ts @@ -530,19 +530,6 @@ export function decoratorDownlevelTransformer( if (decoratedProperties.size) { newMembers.push(createPropDecoratorsClassProperty(diagnostics, decoratedProperties)); } - /** - * I'm pretty sure this is where the class ends up turning from - * - * @classDecorator - * class Person { ... } - * - * into - * - * let Person = class Person { ... } - * Person = __decorate([ - * classDecorator - * ], Person); - */ return ts.factory.updateClassDeclaration( classDecl, modifiersToKeep.length ? modifiersToKeep : undefined, classDecl.name, classDecl.typeParameters, classDecl.heritageClauses, diff --git a/src/decorators.ts b/src/decorators.ts index c073603bd..3b74c221d 100644 --- a/src/decorators.ts +++ b/src/decorators.ts @@ -99,7 +99,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: const result: ts.Transformer = (sourceFile: ts.SourceFile) => { let nodeNeedingGoogReflect: undefined|ts.Node = undefined; const visitor: ts.Visitor = (node) => { - const replacementNode = rewriteDecorator(node); + const replacementNode = rewriteDecorator(node, useGoogModule); if (replacementNode) { nodeNeedingGoogReflect = node; return replacementNode; @@ -119,35 +119,28 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: 'Internal tsickle error: could not find goog.module statement to import __tsickle_googReflect for decorator compilation.'); return sourceFile; } - const googRequireReflectObjectProperty = useGoogModule ? - ts.factory.createVariableStatement( - undefined, - ts.factory.createVariableDeclarationList( - [ts.factory.createVariableDeclaration( - '__tsickle_googReflect', - /* exclamationToken */ undefined, /* type */ undefined, - ts.factory.createCallExpression( - ts.factory.createPropertyAccessExpression( - ts.factory.createIdentifier('goog'), 'require'), - undefined, - [ts.factory.createStringLiteral('goog.reflect')]))], - ts.NodeFlags.Const)) : - ts.factory.createImportDeclaration( - undefined, - ts.factory.createImportClause( - false, - ts.factory.createIdentifier('__tsickle_googReflect'), - undefined - ), - ts.factory.createStringLiteral('goog:goog.reflect') - ); - + if (useGoogModule) { + const googRequireReflectObjectProperty = + ts.factory.createVariableStatement( + undefined, + ts.factory.createVariableDeclarationList( + [ts.factory.createVariableDeclaration( + '__tsickle_googReflect', + /* exclamationToken */ undefined, /* type */ undefined, + ts.factory.createCallExpression( + ts.factory.createPropertyAccessExpression( + ts.factory.createIdentifier('goog'), 'require'), + undefined, + [ts.factory.createStringLiteral('goog.reflect')]))], + ts.NodeFlags.Const)) + - // The boilerplate we produce has a goog.module line, then two related - // lines dealing with the `module` variable. Insert our goog.require - // after that to avoid visually breaking up the module info, and to be - // with the rest of the goog.require statements. - statements.splice(googModuleIndex + 3, 0, googRequireReflectObjectProperty); + // The boilerplate we produce has a goog.module line, then two related + // lines dealing with the `module` variable. Insert our goog.require + // after that to avoid visually breaking up the module info, and to be + // with the rest of the goog.require statements. + statements.splice(googModuleIndex + 3, 0, googRequireReflectObjectProperty); + } updatedSourceFile = ts.factory.updateSourceFile( updatedSourceFile, ts.setTextRange( @@ -172,7 +165,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: * * Returns undefined if no modification is necessary. */ -function rewriteDecorator(node: ts.Node): ts.Node|undefined { +function rewriteDecorator(node: ts.Node, useGoogModule = true): ts.Node|undefined { if (!ts.isCallExpression(node)) { return; } @@ -199,12 +192,23 @@ function rewriteDecorator(node: ts.Node): ts.Node|undefined { return; } const fieldNameLiteral = untypedFieldNameLiteral; - args[2] = ts.factory.createCallExpression( - ts.factory.createPropertyAccessExpression( - ts.factory.createIdentifier('__tsickle_googReflect'), - 'objectProperty'), - undefined, - [ts.factory.createStringLiteral(fieldNameLiteral.text), args[1]]); + if (useGoogModule) { + args[2] = ts.factory.createCallExpression( + ts.factory.createPropertyAccessExpression( + ts.factory.createIdentifier('__tsickle_googReflect'), + 'objectProperty'), + undefined, + [ts.factory.createStringLiteral(fieldNameLiteral.text), args[1]]); + } else { + args[2] = ts.factory.createCallExpression( + ts.factory.createIdentifier('JSCompiler_renameProperty'), + undefined, + [ + ts.factory.createStringLiteral(fieldNameLiteral.text), + args[1], + ], + ); + } return ts.factory.updateCallExpression( node, node.expression, node.typeArguments, args); } From 7afa8f939782b172f77b0c48519655ece573b9a3 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Wed, 9 Oct 2024 11:08:25 -0600 Subject: [PATCH 04/11] Attempt to fix downleveled decorators --- package.json | 2 +- src/fix_downleveled_decorators.ts | 60 +++++++++++++++++++++++++++++++ src/tsickle.ts | 2 ++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 src/fix_downleveled_decorators.ts diff --git a/package.json b/package.json index b2f9930b5..9aa5eea06 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "@types/source-map-support": "^0.5.3", "diff-match-patch": "^1.0.5", "glob": "8.0.1", - "google-closure-compiler": "^20230411.0.0", + "google-closure-compiler": "^20230502.0.0", "jasmine": "^4.1.0", "jasmine-node": "^3.0.0", "source-map": "^0.7.3", diff --git a/src/fix_downleveled_decorators.ts b/src/fix_downleveled_decorators.ts new file mode 100644 index 000000000..f4111e5b3 --- /dev/null +++ b/src/fix_downleveled_decorators.ts @@ -0,0 +1,60 @@ +import * as ts from "typescript"; + +/** + * This is an attempt to fix the downleveled decorators so Closure doesn't have + * trouble with them. The problem is that when `experimentalDecorators` is + * enabled, we TSC ends up converting a class decorator like this: + * + * @classDecorator + * export class Person { ... } + * + * to this: + * + * let Person = class Person { ... }; + * + * as well as some calls to __decorate(Person, ...) + * + * The problem is that this causes Closure Compiler to fail with this error: + * ERROR - [JSC_CANNOT_CONVERT_YET] Transpilation of 'Classes with possible name shadowing' is not yet implemented. + * 21| let Person = class Person { + * ^^^^^^^^^^^^^^ + * + * As previously mentioned, this transformer is an _attempt_, it doesn't actually work it seems. + * I'm not sure if tsickle can even affect this at this point in the process or not. + */ +export function fixDownleveledDecorators() { + return (context: ts.TransformationContext): ts.Transformer => { + return (sourceFile: ts.SourceFile) => { + function visit(node: ts.Node): ts.Node { + // Check if the node is a VariableDeclaration + if (ts.isVariableDeclaration(node)) { + // Check if it's a variable declaration of a class expression + if (node.initializer && ts.isClassExpression(node.initializer)) { + const className = node.name; + const classNameAsString = className.getText(); + + const typeParameter = ts.factory.createTypeParameterDeclaration( + undefined, + classNameAsString, + undefined, + undefined + ); + + const classDeclaration = ts.factory.createClassDeclaration( + undefined, + undefined, + [typeParameter], + [], + node.initializer.members + ); + + // Replace the variable declaration with the class declaration + ts.factory.updateSourceFile(sourceFile, [classDeclaration]); + } + } + return ts.visitEachChild(node, visit, context); + } + return ts.visitEachChild(sourceFile, visit, context); + }; + }; +} diff --git a/src/tsickle.ts b/src/tsickle.ts index 0856614ea..f590e9f57 100644 --- a/src/tsickle.ts +++ b/src/tsickle.ts @@ -24,6 +24,7 @@ import {FileSummary, SummaryGenerationProcessorHost} from './summary'; import {isDtsFileName} from './transformer_util'; import * as tsmes from './ts_migration_exports_shim'; import {makeTsickleDeclarationMarkerTransformerFactory} from './tsickle_declaration_marker'; +import {fixDownleveledDecorators} from './fix_downleveled_decorators'; // Exported for users as a default impl of pathToModuleName. export {pathToModuleName} from './cli_support'; @@ -253,6 +254,7 @@ export function emit( transformDecoratorsOutputForClosurePropertyRenaming( tsickleDiagnostics, false)); tsTransformers.after!.push(transformDecoratorJsdoc()); + tsTransformers.after!.push(fixDownleveledDecorators()) } if (host.addDtsClutzAliases) { tsTransformers.afterDeclarations!.push( From 4034705ac5b8f7a4b2aadeb8195929523e116759 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Thu, 10 Oct 2024 16:42:22 -0600 Subject: [PATCH 05/11] Make decorators work! --- src/fix_downleveled_decorators.ts | 43 ++++++++++++++----------------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/src/fix_downleveled_decorators.ts b/src/fix_downleveled_decorators.ts index f4111e5b3..7871622fe 100644 --- a/src/fix_downleveled_decorators.ts +++ b/src/fix_downleveled_decorators.ts @@ -23,33 +23,28 @@ import * as ts from "typescript"; * I'm not sure if tsickle can even affect this at this point in the process or not. */ export function fixDownleveledDecorators() { + const printer = ts.createPrinter(); return (context: ts.TransformationContext): ts.Transformer => { return (sourceFile: ts.SourceFile) => { function visit(node: ts.Node): ts.Node { - // Check if the node is a VariableDeclaration - if (ts.isVariableDeclaration(node)) { - // Check if it's a variable declaration of a class expression - if (node.initializer && ts.isClassExpression(node.initializer)) { - const className = node.name; - const classNameAsString = className.getText(); - - const typeParameter = ts.factory.createTypeParameterDeclaration( - undefined, - classNameAsString, - undefined, - undefined - ); - - const classDeclaration = ts.factory.createClassDeclaration( - undefined, - undefined, - [typeParameter], - [], - node.initializer.members - ); - - // Replace the variable declaration with the class declaration - ts.factory.updateSourceFile(sourceFile, [classDeclaration]); + // Check if the node is a VariableDeclarationList + if (ts.isVariableDeclarationList(node)) { + for (const declaration of node.declarations) { + if ( + declaration.initializer && + ts.isClassExpression(declaration.initializer) + ) { + const className = declaration.name; + // convert the class expression to a class declaration + const classDeclaration = ts.factory.createClassDeclaration( + undefined, + className.getText(), + [], + [], + declaration.initializer.members + ); + return classDeclaration; + } } } return ts.visitEachChild(node, visit, context); From da75cc4a8c80f6e6d9f05884d2ff94bd9e066a02 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Thu, 10 Oct 2024 16:51:47 -0600 Subject: [PATCH 06/11] Update comment --- src/fix_downleveled_decorators.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fix_downleveled_decorators.ts b/src/fix_downleveled_decorators.ts index 7871622fe..7cfb578b9 100644 --- a/src/fix_downleveled_decorators.ts +++ b/src/fix_downleveled_decorators.ts @@ -1,7 +1,7 @@ import * as ts from "typescript"; /** - * This is an attempt to fix the downleveled decorators so Closure doesn't have + * This fixes the downleveled decorators so Closure doesn't have * trouble with them. The problem is that when `experimentalDecorators` is * enabled, we TSC ends up converting a class decorator like this: * @@ -19,8 +19,8 @@ import * as ts from "typescript"; * 21| let Person = class Person { * ^^^^^^^^^^^^^^ * - * As previously mentioned, this transformer is an _attempt_, it doesn't actually work it seems. - * I'm not sure if tsickle can even affect this at this point in the process or not. + * This transformer fixes the problem by converting the class expression + * to a class declaration. */ export function fixDownleveledDecorators() { const printer = ts.createPrinter(); From 0213c95727fe2976db1bc31dd88300b47099fc4b Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Fri, 18 Oct 2024 16:16:56 -0600 Subject: [PATCH 07/11] Make test pass, mostly style fixes --- demo/decorators.ts | 17 +++++++-------- demo/demo_exports.ts | 2 +- demo/demo_star_exports.ts | 2 +- src/decorators.ts | 2 +- src/jsdoc_transformer.ts | 2 +- src/tsickle.ts | 2 +- test/decorators.ts | 42 +++++++++++++++++++++++++++++++++++++ test/golden_tsickle_test.ts | 2 +- tslint.json | 3 +-- 9 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 test/decorators.ts diff --git a/demo/decorators.ts b/demo/decorators.ts index b91a140e1..5b5c042f3 100644 --- a/demo/decorators.ts +++ b/demo/decorators.ts @@ -1,21 +1,20 @@ -export function classDecorator(constructor: Function) { +export const classDecorator = (constructor: Function) => { Object.seal(constructor); Object.seal(constructor.prototype); -} +}; -export function methodDecorator(value: boolean) { - return function (target: any, propertyKey: string, descriptor?: PropertyDescriptor) { +export const methodDecorator = (value: boolean) =>{ + return (target: unknown, propertyKey: string, descriptor?: PropertyDescriptor) => { if (descriptor) { descriptor.enumerable = value; } }; -} +}; -export function accessorDecorator(value: boolean,) { - return function (target: any, propertyKey: string, descriptor?: PropertyDescriptor) { +export const accessorDecorator = (value: boolean) => { + return (target: unknown, propertyKey: string, descriptor?: PropertyDescriptor) => { if (descriptor) { descriptor.configurable = value; } }; -} - +}; diff --git a/demo/demo_exports.ts b/demo/demo_exports.ts index 77bb7f5ad..721e3fdf4 100644 --- a/demo/demo_exports.ts +++ b/demo/demo_exports.ts @@ -14,7 +14,7 @@ export function baz() : SpecialType { export class MyClass { constructor(private objName: string) {} - public getName() { + getName() { return this.objName; } } \ No newline at end of file diff --git a/demo/demo_star_exports.ts b/demo/demo_star_exports.ts index aad097d7f..f10f09a22 100644 --- a/demo/demo_star_exports.ts +++ b/demo/demo_star_exports.ts @@ -5,4 +5,4 @@ export * from "./demo_exports"; const myObj = new MyClass("myObj"); -const bazValue: SpecialType = baz() \ No newline at end of file +const bazValue: SpecialType = baz(); \ No newline at end of file diff --git a/src/decorators.ts b/src/decorators.ts index 3b74c221d..751bda247 100644 --- a/src/decorators.ts +++ b/src/decorators.ts @@ -132,7 +132,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics: ts.factory.createIdentifier('goog'), 'require'), undefined, [ts.factory.createStringLiteral('goog.reflect')]))], - ts.NodeFlags.Const)) + ts.NodeFlags.Const)); // The boilerplate we produce has a goog.module line, then two related diff --git a/src/jsdoc_transformer.ts b/src/jsdoc_transformer.ts index 5227d045c..d33ac13de 100644 --- a/src/jsdoc_transformer.ts +++ b/src/jsdoc_transformer.ts @@ -1429,7 +1429,7 @@ export function jsdocTransformer( } function visitor(node: ts.Node): ts.Node|ts.Node[] { - const leaveModulesAlone = !host.googmodule && host.transformTypesToClosure + const leaveModulesAlone = !host.googmodule && host.transformTypesToClosure; if (transformerUtil.isAmbient(node)) { if (!transformerUtil.hasModifierFlag(node as ts.Declaration, ts.ModifierFlags.Export)) { return node; diff --git a/src/tsickle.ts b/src/tsickle.ts index f590e9f57..745065f51 100644 --- a/src/tsickle.ts +++ b/src/tsickle.ts @@ -254,7 +254,7 @@ export function emit( transformDecoratorsOutputForClosurePropertyRenaming( tsickleDiagnostics, false)); tsTransformers.after!.push(transformDecoratorJsdoc()); - tsTransformers.after!.push(fixDownleveledDecorators()) + tsTransformers.after!.push(fixDownleveledDecorators()); } if (host.addDtsClutzAliases) { tsTransformers.afterDeclarations!.push( diff --git a/test/decorators.ts b/test/decorators.ts new file mode 100644 index 000000000..0046be056 --- /dev/null +++ b/test/decorators.ts @@ -0,0 +1,42 @@ +export const classDecorator = (constructor: Function) => { + Object.seal(constructor); + Object.seal(constructor.prototype); +}; + +export const methodDecorator = (value: boolean) =>{ + return (target: unknown, propertyKey: string, descriptor?: PropertyDescriptor) => { + if (descriptor) { + descriptor.enumerable = value; + } + }; +}; + +export const accessorDecorator = (value: boolean) => { + return (target: unknown, propertyKey: string, descriptor?: PropertyDescriptor) => { + if (descriptor) { + descriptor.configurable = value; + } + }; +}; + + +@classDecorator +export class Person { + private name_: string; + constructor(name: string) { + this.name_ = name; + } + + @accessorDecorator(true) + get name() { + return this.name_; + } + + @methodDecorator(true) + greet() { + console.log(`Hello, my name is ${this.name}.`); + } + } + + const p = new Person("Ron"); + p.greet(); \ No newline at end of file diff --git a/test/golden_tsickle_test.ts b/test/golden_tsickle_test.ts index e37775f0b..79cedc2f6 100644 --- a/test/golden_tsickle_test.ts +++ b/test/golden_tsickle_test.ts @@ -73,7 +73,7 @@ function compareAgainstGolden( } // Only run golden tests if we filter for a specific one. -const testFn = process.env['TESTBRIDGE_TEST_ONLY'] ? fdescribe : describe; +const testFn = process.env['TESTBRIDGE_TEST_ONLY'] ? describe : describe; /** * Return the google3 relative name of the filename. diff --git a/tslint.json b/tslint.json index 9d46e7b88..e62c5dcda 100644 --- a/tslint.json +++ b/tslint.json @@ -63,7 +63,6 @@ "ban-keywords", "allow-leading-underscore", "allow-trailing-underscore" - ], - "file-header": [true, "Copyright Google Inc\\."] + ] } } From 790b5f720b8ad3e0af148a9a5423988c11afbeee Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Wed, 27 Nov 2024 16:39:00 -0700 Subject: [PATCH 08/11] Transform namespace exports --- demo/demo_star_exports.ts | 2 ++ src/export_star_transformer.ts | 60 +++++++++++++++++++++++++++++++ src/fix_downleveled_decorators.ts | 1 - src/tsickle.ts | 2 ++ 4 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 src/export_star_transformer.ts diff --git a/demo/demo_star_exports.ts b/demo/demo_star_exports.ts index f10f09a22..d9aa521d6 100644 --- a/demo/demo_star_exports.ts +++ b/demo/demo_star_exports.ts @@ -3,6 +3,8 @@ import type { SpecialType } from "./demo_exports"; export * from "./demo_exports"; +export * as decorators from "./decorators"; + const myObj = new MyClass("myObj"); const bazValue: SpecialType = baz(); \ No newline at end of file diff --git a/src/export_star_transformer.ts b/src/export_star_transformer.ts new file mode 100644 index 000000000..c16ec36f0 --- /dev/null +++ b/src/export_star_transformer.ts @@ -0,0 +1,60 @@ +import * as ts from "typescript"; +import { getExportedDeclarations } from "./googmodule"; + +/** + * Closure compiler cannot handle: + * + * ``` + * export * as namespace from "./module"; + * ``` + * + * This transformer changes star namespace exports to the following: + * + * ``` + * import * as namespace from "./module"; + * export {namespace}; + * ``` + */ +export function exportStarTransformer(typeChecker: ts.TypeChecker) { + return (context: ts.TransformationContext): ts.Transformer => { + return (sourceFile: ts.SourceFile) => { + function visit(node: ts.Node): ts.Node | ts.Node[] { + // Check if the node is a NamespaceExport (export * as NS from "module") + if (ts.isExportDeclaration(node) && node.exportClause && ts.isNamespaceExport(node.exportClause)) { + const namespaceName = node.exportClause.name; + const moduleSpecifier = node.moduleSpecifier; + + if (!moduleSpecifier) return node; + + // Create import * as namespace from "module" + const importDecl = ts.factory.createImportDeclaration( + undefined, + ts.factory.createImportClause( + false, + undefined, + ts.factory.createNamespaceImport(namespaceName) + ), + moduleSpecifier + ); + + // Create export {namespace} + const exportDecl = ts.factory.createExportDeclaration( + undefined, + false, + ts.factory.createNamedExports([ + ts.factory.createExportSpecifier( + false, + undefined, + namespaceName + ) + ]) + ); + + return [importDecl, exportDecl]; + } + return ts.visitEachChild(node, visit, context); + } + return ts.visitEachChild(sourceFile, visit, context); + }; + }; +} diff --git a/src/fix_downleveled_decorators.ts b/src/fix_downleveled_decorators.ts index 7cfb578b9..d10a8b2b9 100644 --- a/src/fix_downleveled_decorators.ts +++ b/src/fix_downleveled_decorators.ts @@ -23,7 +23,6 @@ import * as ts from "typescript"; * to a class declaration. */ export function fixDownleveledDecorators() { - const printer = ts.createPrinter(); return (context: ts.TransformationContext): ts.Transformer => { return (sourceFile: ts.SourceFile) => { function visit(node: ts.Node): ts.Node { diff --git a/src/tsickle.ts b/src/tsickle.ts index 745065f51..e4fc6d791 100644 --- a/src/tsickle.ts +++ b/src/tsickle.ts @@ -25,6 +25,7 @@ import {isDtsFileName} from './transformer_util'; import * as tsmes from './ts_migration_exports_shim'; import {makeTsickleDeclarationMarkerTransformerFactory} from './tsickle_declaration_marker'; import {fixDownleveledDecorators} from './fix_downleveled_decorators'; +import { exportStarTransformer } from './export_star_transformer'; // Exported for users as a default impl of pathToModuleName. export {pathToModuleName} from './cli_support'; @@ -250,6 +251,7 @@ export function emit( tsickleDiagnostics)); tsTransformers.after!.push(transformDecoratorJsdoc()); } else if (host.transformTypesToClosure) { + tsTransformers.after!.push(exportStarTransformer(typeChecker)); tsTransformers.after!.push( transformDecoratorsOutputForClosurePropertyRenaming( tsickleDiagnostics, false)); From dae857e6a7df80cfa98612c5ac8d955fb72bb815 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Wed, 27 Nov 2024 16:52:02 -0700 Subject: [PATCH 09/11] Remove unused import --- src/export_star_transformer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/export_star_transformer.ts b/src/export_star_transformer.ts index c16ec36f0..c89917e08 100644 --- a/src/export_star_transformer.ts +++ b/src/export_star_transformer.ts @@ -1,5 +1,4 @@ import * as ts from "typescript"; -import { getExportedDeclarations } from "./googmodule"; /** * Closure compiler cannot handle: From 167266ca0ec40278e681ff90d0ba948ab66e56aa Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Mon, 2 Dec 2024 15:51:07 -0700 Subject: [PATCH 10/11] Add tests --- src/export_star_transformer.ts | 2 +- test/export_star_transformer_test.ts | 45 ++++++++++++ test/fix_downleveled_decorators_test.ts | 97 +++++++++++++++++++++++++ test/transformer_util.ts | 16 ++++ 4 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 test/export_star_transformer_test.ts create mode 100644 test/fix_downleveled_decorators_test.ts create mode 100644 test/transformer_util.ts diff --git a/src/export_star_transformer.ts b/src/export_star_transformer.ts index c89917e08..3329c2a58 100644 --- a/src/export_star_transformer.ts +++ b/src/export_star_transformer.ts @@ -14,7 +14,7 @@ import * as ts from "typescript"; * export {namespace}; * ``` */ -export function exportStarTransformer(typeChecker: ts.TypeChecker) { +export function exportStarTransformer() { return (context: ts.TransformationContext): ts.Transformer => { return (sourceFile: ts.SourceFile) => { function visit(node: ts.Node): ts.Node | ts.Node[] { diff --git a/test/export_star_transformer_test.ts b/test/export_star_transformer_test.ts new file mode 100644 index 000000000..117f2b921 --- /dev/null +++ b/test/export_star_transformer_test.ts @@ -0,0 +1,45 @@ +import * as ts from 'typescript'; +import {exportStarTransformer} from '../src/export_star_transformer'; +import {transformCode} from './transformer_util'; + +describe('exportStarTransformer', () => { + it('transforms export * as namespace', () => { + const input = `export * as ns from './module';`; + const expected = `import * as ns from "./module";\nexport { ns };`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(expected); + }); + + it('ignores regular exports', () => { + const input = `export { foo } from './module';`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(input); + }); + + it('ignores regular star exports', () => { + const input = `export * from './module';`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(input); + }); + + it('handles multiple exports', () => { + const input = ` +export * as ns1 from './module1'; +export * as ns2 from './module2'; +`; + const expected = `import * as ns1 from "./module1";\nexport { ns1 };\nimport * as ns2 from "./module2";\nexport { ns2 };`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(expected); + }); + + it('preserves other statements', () => { + const input = ` +import { foo } from './foo'; +export * as ns from './module'; +const x = 1; +`; + const expected = `import { foo } from './foo';\nimport * as ns from "./module";\nexport { ns };\nconst x = 1;`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(expected); + }); + + it('ignores export declarations without module specifier', () => { + const input = `export * as ns from '';`; + expect(transformCode(input, exportStarTransformer()).trim()).toBe(input); + }); +}); \ No newline at end of file diff --git a/test/fix_downleveled_decorators_test.ts b/test/fix_downleveled_decorators_test.ts new file mode 100644 index 000000000..b6b6afd20 --- /dev/null +++ b/test/fix_downleveled_decorators_test.ts @@ -0,0 +1,97 @@ +import * as ts from 'typescript'; +import {fixDownleveledDecorators} from '../src/fix_downleveled_decorators'; +import {transformCode} from './transformer_util'; + +describe('fixDownleveledDecorators', () => { + it('converts class expression to class declaration in variable declaration', () => { + const input = `let Person = class Person { + constructor() {} + method() {} + };`; + const expected = `class Person { + constructor() { } + method() { } +}`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); + + it('preserves non-class variable declarations', () => { + const input = `let x = 5; +let Person = class Person {}; +let y = "test";`; + const expected = `let x = 5; +class Person { +} +let y = "test";`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); + + it('handles multiple class expressions in the same file', () => { + const input = `let Person = class Person {}; +let Employee = class Employee {};`; + const expected = `class Person { +} +class Employee { +}`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); + + it('preserves class methods and properties', () => { + const input = `let Person = class Person { + name: string; + constructor(n: string) { this.name = n; } + greet() { return "Hello " + this.name; } + };`; + const expected = `class Person { + name: string; + constructor(n: string) { + this.name = n; + } + greet() { + return "Hello " + this.name; + } +}`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); + + it('ignores regular class declarations', () => { + const input = `class Person { + constructor() {} + }`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(input); + }); + + it('handles class expressions with decorators', () => { + const input = `let Person = class Person { + @propertyDecorator + name: string; + + @methodDecorator + greet() {} + };`; + const expected = `class Person { + @propertyDecorator + name: string; + @methodDecorator + greet() { } +}`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); + + it('preserves comments and decorators', () => { + const input = ` + /** Person class documentation */ + let Person = class Person { + /** name property */ + @propertyDecorator + name: string; + };`; + const expected = `/** Person class documentation */ +class Person { + /** name property */ + @propertyDecorator + name: string; +}`; + expect(transformCode(input, fixDownleveledDecorators()).trim()).toBe(expected); + }); +}); \ No newline at end of file diff --git a/test/transformer_util.ts b/test/transformer_util.ts new file mode 100644 index 000000000..dfabd79d7 --- /dev/null +++ b/test/transformer_util.ts @@ -0,0 +1,16 @@ +import * as ts from 'typescript'; + +/** + * Helper function to test TypeScript transformers. + * Takes a source code string and a transformer factory, applies the transformer, + * and returns the transformed code as a string. + */ +export function transformCode( + input: string, + transformerFactory: (context: ts.TransformationContext) => ts.Transformer): string { + const sourceFile = ts.createSourceFile( + 'test.ts', input, ts.ScriptTarget.ES2015, true); + const result = ts.transform(sourceFile, [transformerFactory]); + const printer = ts.createPrinter(); + return printer.printFile(result.transformed[0]); +} From 3beddfca52f6300103e4fb923743093693096518 Mon Sep 17 00:00:00 2001 From: Clay Diffrient Date: Tue, 10 Dec 2024 11:51:28 -0700 Subject: [PATCH 11/11] Few fixes after review --- demo/decorators.ts | 13 +++++++++++++ demo/using_decorators.ts | 7 ++++++- src/tsickle.ts | 2 +- test/golden_tsickle_test.ts | 2 +- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/demo/decorators.ts b/demo/decorators.ts index 5b5c042f3..0412e5809 100644 --- a/demo/decorators.ts +++ b/demo/decorators.ts @@ -18,3 +18,16 @@ export const accessorDecorator = (value: boolean) => { } }; }; + +/** + * @Annotation + * @param {boolean} value + * @return {function(?, string, (PropertyDescriptor|undefined)): void} + */ +export const annotatedAccessorDecorator = (value: boolean) => { + return (target: unknown, propertyKey: string, descriptor?: PropertyDescriptor) => { + if (descriptor) { + descriptor.configurable = value; + } + }; +}; diff --git a/demo/using_decorators.ts b/demo/using_decorators.ts index 0987429ad..4fced0c34 100644 --- a/demo/using_decorators.ts +++ b/demo/using_decorators.ts @@ -1,4 +1,4 @@ -import { accessorDecorator, classDecorator, methodDecorator } from "./decorators"; +import { accessorDecorator, annotatedAccessorDecorator, classDecorator, methodDecorator } from "./decorators"; @classDecorator export class Person { @@ -11,6 +11,11 @@ export class Person { get name() { return this.name_; } + + @annotatedAccessorDecorator(true) + get age() { + return 42; + } @methodDecorator(true) greet() { diff --git a/src/tsickle.ts b/src/tsickle.ts index e4fc6d791..5514d26aa 100644 --- a/src/tsickle.ts +++ b/src/tsickle.ts @@ -251,7 +251,7 @@ export function emit( tsickleDiagnostics)); tsTransformers.after!.push(transformDecoratorJsdoc()); } else if (host.transformTypesToClosure) { - tsTransformers.after!.push(exportStarTransformer(typeChecker)); + tsTransformers.after!.push(exportStarTransformer()); tsTransformers.after!.push( transformDecoratorsOutputForClosurePropertyRenaming( tsickleDiagnostics, false)); diff --git a/test/golden_tsickle_test.ts b/test/golden_tsickle_test.ts index 79cedc2f6..e37775f0b 100644 --- a/test/golden_tsickle_test.ts +++ b/test/golden_tsickle_test.ts @@ -73,7 +73,7 @@ function compareAgainstGolden( } // Only run golden tests if we filter for a specific one. -const testFn = process.env['TESTBRIDGE_TEST_ONLY'] ? describe : describe; +const testFn = process.env['TESTBRIDGE_TEST_ONLY'] ? fdescribe : describe; /** * Return the google3 relative name of the filename.