From e8763d618ceb7d661e18a4495f6b937fafd0553d Mon Sep 17 00:00:00 2001 From: Timofei Iatsenko Date: Mon, 18 Oct 2021 17:05:30 +0200 Subject: [PATCH] fix(plugin) downlevel implicit registerEnumType import in CommonJS When code is downleveled to ES5 it automatically change all usages of imported members except those which was added by transform. This breaks `registerEnumType()` calls added by transform. The solution is create an explicit dedicated import only for this method in the transform and not relay on exisitng imports at all. --- lib/plugin/utils/ast-utils.ts | 25 +++++ lib/plugin/visitors/model-class.visitor.ts | 117 +++++++++++---------- tests/plugin/model-class-visitor.spec.ts | 60 ++++++++++- 3 files changed, 147 insertions(+), 55 deletions(-) diff --git a/lib/plugin/utils/ast-utils.ts b/lib/plugin/utils/ast-utils.ts index 27f51ae5c..3be8bc7be 100644 --- a/lib/plugin/utils/ast-utils.ts +++ b/lib/plugin/utils/ast-utils.ts @@ -234,6 +234,31 @@ export function hasImport(sf: ts.SourceFile, what: string): boolean { return false; } +export function createImportEquals( + f: ts.NodeFactory, + identifier: ts.Identifier | string, + from: string, +): ts.ImportEqualsDeclaration { + const [major, minor] = ts.versionMajorMinor?.split('.').map((x) => +x); + + if (major == 4 && minor >= 2) { + // support TS v4.2+ + return f.createImportEqualsDeclaration( + undefined, + undefined, + false, + identifier, + f.createExternalModuleReference(f.createStringLiteral(from)), + ); + } + return (f.createImportEqualsDeclaration as any)( + undefined, + undefined, + identifier, + f.createExternalModuleReference(f.createStringLiteral(from)), + ); +} + export function createNamedImport( f: ts.NodeFactory, what: string[], diff --git a/lib/plugin/visitors/model-class.visitor.ts b/lib/plugin/visitors/model-class.visitor.ts index ad4fe08e7..bc35b8834 100644 --- a/lib/plugin/visitors/model-class.visitor.ts +++ b/lib/plugin/visitors/model-class.visitor.ts @@ -1,4 +1,5 @@ import * as ts from 'typescript'; +import { ModuleKind } from 'typescript'; import { HideField, ObjectType, @@ -16,13 +17,14 @@ import { hasDecorators, hasModifiers, getDecoratorName, - createNamedImport, isCallExpressionOf, serializePrimitiveObjectToAst, safelyMergeObjects, hasJSDocTags, PrimitiveObject, + createImportEquals, hasImport, + createNamedImport, } from '../utils/ast-utils'; import { getTypeReferenceAsString, @@ -52,6 +54,8 @@ function capitalizeFirstLetter(word: string) { export class ModelClassVisitor { inlineEnumsMap: { name: string; values: { [name: string]: string } }[]; enumsMetadata: Map; + packageVarIdentifier: ts.Identifier; + isCommonJs: boolean; visit( sourceFile: ts.SourceFile, @@ -62,9 +66,13 @@ export class ModelClassVisitor { this.inlineEnumsMap = []; this.enumsMetadata = new Map(); + this.isCommonJs = ctx.getCompilerOptions().module === ModuleKind.CommonJS; + const typeChecker = program.getTypeChecker(); const factory = ctx.factory; + this.packageVarIdentifier = factory.createUniqueName('nestjs_graphql'); + const visitNode = (node: ts.Node): ts.Node => { if ( ts.isClassDeclaration(node) && @@ -112,13 +120,24 @@ export class ModelClassVisitor { const implicitEnumsStatements = this.createImplicitEnums(factory); - if ( - (implicitEnumsStatements.length || this.enumsMetadata.size) && - !hasImport(sourceFile, 'registerEnumType') - ) { - importStatements.push( - createNamedImport(factory, ['registerEnumType'], '@nestjs/graphql'), - ); + if (implicitEnumsStatements.length || this.enumsMetadata.size) { + if (this.isCommonJs) { + importStatements.push( + createImportEquals( + factory, + this.packageVarIdentifier, + '@nestjs/graphql', + ), + ); + } else if (!hasImport(sourceFile, 'registerEnumType')) { + importStatements.push( + createNamedImport( + factory, + ['registerEnumType'], + '@nestjs/graphql', + ), + ); + } } const existingStatements = Array.from(visitedNode.statements); @@ -182,21 +201,15 @@ export class ModelClassVisitor { valuesMap: metadata.properties, }; - return f.createExpressionStatement( - f.createCallExpression( - f.createIdentifier('registerEnumType'), - undefined, - [ - // create enum itself as object literal - f.createIdentifier(metadata.name), - // create an options with name of enum - serializePrimitiveObjectToAst( - f, - registerEnumTypeOptions as unknown as PrimitiveObject, - ), - ], + return this.createRegisterEnumTypeFnCall(f, [ + // create enum itself as object literal + f.createIdentifier(metadata.name), + // create an options with name of enum + serializePrimitiveObjectToAst( + f, + registerEnumTypeOptions as unknown as PrimitiveObject, ), - ); + ]); } private amendCreateUnionTypeCall(f: ts.NodeFactory, node: ts.CallExpression) { @@ -330,20 +343,36 @@ export class ModelClassVisitor { return values; } + private createRegisterEnumTypeFnCall( + f: ts.NodeFactory, + argumentsArray: ts.Expression[], + ) { + const FN_NAME = 'registerEnumType'; + let callee: ts.Expression; + + // https://stackoverflow.com/questions/69617562/adding-a-function-call-in-typescript-transform-compiler-api + if (this.isCommonJs) { + callee = f.createPropertyAccessExpression( + this.packageVarIdentifier, + FN_NAME, + ); + } else { + callee = f.createIdentifier(FN_NAME); + } + + return f.createExpressionStatement( + f.createCallExpression(callee, undefined, argumentsArray), + ); + } + private createImplicitEnums(f: ts.NodeFactory): ts.ExpressionStatement[] { return this.inlineEnumsMap.map(({ name, values }) => { - return f.createExpressionStatement( - f.createCallExpression( - f.createIdentifier('registerEnumType'), - undefined, - [ - // create enum itself as object literal - serializePrimitiveObjectToAst(f, values), - // create an options with name of enum - serializePrimitiveObjectToAst(f, { name }), - ], - ), - ); + return this.createRegisterEnumTypeFnCall(f, [ + // create enum itself as object literal + serializePrimitiveObjectToAst(f, values), + // create an options with name of enum + serializePrimitiveObjectToAst(f, { name }), + ]); }); } @@ -570,26 +599,8 @@ export class ModelClassVisitor { return []; } - const [major, minor] = ts.versionMajorMinor?.split('.').map((x) => +x); - const IMPORT_PREFIX = 'eager_import_'; - return Array.from(importsToAdd).map((path, index) => { - if (major == 4 && minor >= 2) { - // support TS v4.2+ - return f.createImportEqualsDeclaration( - undefined, - undefined, - false, - IMPORT_PREFIX + index, - f.createExternalModuleReference(f.createStringLiteral(path)), - ); - } - return (f.createImportEqualsDeclaration as any)( - undefined, - undefined, - IMPORT_PREFIX + index, - f.createExternalModuleReference(f.createStringLiteral(path)), - ); + return createImportEquals(f, 'eager_import_' + index, path); }); } } diff --git a/tests/plugin/model-class-visitor.spec.ts b/tests/plugin/model-class-visitor.spec.ts index 57005df8b..0a1b014fd 100644 --- a/tests/plugin/model-class-visitor.spec.ts +++ b/tests/plugin/model-class-visitor.spec.ts @@ -1,4 +1,5 @@ import * as ts from 'typescript'; +import { ModuleKind } from 'typescript'; import { before } from '../../lib/plugin/compiler-plugin'; import { createCatDtoAltText, @@ -465,7 +466,7 @@ registerEnumType(Status, { name: \\"Status\\", description: \\"Description for E `); }); - it('Should not add additional import if there is one', () => { + it('Should not add additional import if there is one in ES Modules', () => { const source = ` import { registerEnumType, otherPackage } from '@nestjs/graphql'; @@ -486,7 +487,11 @@ otherPackage(); registerEnumType(Status2, {name: 'Status2'}); `; - const actual = transpile(source, { autoRegisterEnums: true }); + const actual = transpile( + source, + { autoRegisterEnums: true }, + { module: ModuleKind.ES2015 }, + ); expect(actual).toMatchInlineSnapshot(` "import { registerEnumType, otherPackage } from '@nestjs/graphql'; var Status; @@ -506,6 +511,57 @@ var Status2; otherPackage(); registerEnumType(Status2, { name: 'Status2' }); " +`); + }); + + it('Should create eager namespaced import and call registerEnumType from this import in CommonJs', () => { + const source = ` +import { registerEnumType, otherPackage } from '@nestjs/graphql'; + +enum Status { + ENABLED, + DISABLED +} + +/** +* @private +*/ +enum Status2 { + ENABLED, + DISABLED +} + +otherPackage(); +registerEnumType(Status2, {name: 'Status2'}); +`; + + const actual = transpile( + source, + { autoRegisterEnums: true }, + { module: ModuleKind.CommonJS }, + ); + expect(actual).toMatchInlineSnapshot(` +"\\"use strict\\"; +Object.defineProperty(exports, \\"__esModule\\", { value: true }); +var nestjs_graphql_1 = require(\\"@nestjs/graphql\\"); +var graphql_1 = require(\\"@nestjs/graphql\\"); +var Status; +(function (Status) { + Status[Status[\\"ENABLED\\"] = 0] = \\"ENABLED\\"; + Status[Status[\\"DISABLED\\"] = 1] = \\"DISABLED\\"; +})(Status || (Status = {})); +nestjs_graphql_1.registerEnumType(Status, { name: \\"Status\\", valuesMap: {} }); +/** +* @private +*/ +var Status2; +(function (Status2) { + Status2[Status2[\\"ENABLED\\"] = 0] = \\"ENABLED\\"; + Status2[Status2[\\"DISABLED\\"] = 1] = \\"DISABLED\\"; +})(Status2 || (Status2 = {})); +(0, graphql_1.otherPackage)(); +(0, graphql_1.registerEnumType)(Status2, { name: 'Status2' }); +" `); }); });