Skip to content

Commit 5d59b38

Browse files
refactor(plugin) remove nested types introspection, refactor
Nested types introspection was useless because runtime did not understand this code. There is no way to use "anonymous" type literals in graphql schema Add tests for replaceImportPath util, because current test suite didn't cover all branches in it.
1 parent d84e42d commit 5d59b38

File tree

4 files changed

+91
-77
lines changed

4 files changed

+91
-77
lines changed

lib/plugin/utils/plugin-utils.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ export function isPromiseOrObservable(type: string) {
9999

100100
export function replaceImportPath(typeReference: string, fileName: string) {
101101
if (!typeReference.includes('import')) {
102-
return typeReference;
102+
return { typeReference, importPath: null };
103103
}
104-
let importPath = /\(\"([^)]).+(\")/.exec(typeReference)[0];
104+
let importPath = /\("([^)]).+(")/.exec(typeReference)[0];
105105
if (!importPath) {
106-
return undefined;
106+
return { typeReference: undefined, importPath: null };
107107
}
108108
importPath = convertPath(importPath);
109109
importPath = importPath.slice(2, importPath.length - 1);
@@ -134,7 +134,9 @@ export function replaceImportPath(typeReference: string, fileName: string) {
134134
}
135135

136136
typeReference = typeReference.replace(importPath, relativePath);
137-
return typeReference.replace('import', 'require');
137+
typeReference = typeReference.replace('import', 'require');
138+
139+
return { typeReference, importPath: relativePath };
138140
}
139141

140142
export function isDynamicallyAdded(identifier: ts.Node) {

lib/plugin/visitors/model-class.visitor.ts

Lines changed: 31 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ import {
3333
import { EnumMetadataValuesMapOptions } from '../../schema-builder/metadata';
3434
import { EnumOptions } from '../../type-factories';
3535

36-
const importsToAddPerFile = new Map<string, Set<string>>();
37-
3836
const ALLOWED_DECORATORS = [
3937
ObjectType.name,
4038
InterfaceType.name,
@@ -52,6 +50,8 @@ function capitalizeFirstLetter(word: string) {
5250
}
5351

5452
export class ModelClassVisitor {
53+
importsToAdd: Set<string>;
54+
5555
inlineEnumsMap: { name: string; values: { [name: string]: string } }[];
5656
enumsMetadata: Map<ts.EnumDeclaration, EnumMetadata>;
5757
packageVarIdentifier: ts.Identifier;
@@ -65,7 +65,7 @@ export class ModelClassVisitor {
6565
) {
6666
this.inlineEnumsMap = [];
6767
this.enumsMetadata = new Map();
68-
68+
this.importsToAdd = new Set<string>();
6969
this.isCommonJs = ctx.getCompilerOptions().module === ModuleKind.CommonJS;
7070

7171
const typeChecker = program.getTypeChecker();
@@ -113,10 +113,8 @@ export class ModelClassVisitor {
113113
} else if (ts.isSourceFile(node)) {
114114
const visitedNode = ts.visitEachChild(node, visitNode, ctx);
115115

116-
const importStatements: ts.Statement[] = this.createEagerImports(
117-
factory,
118-
node.fileName,
119-
);
116+
const importStatements: ts.Statement[] =
117+
this.createEagerImports(factory);
120118

121119
const implicitEnumsStatements = this.createImplicitEnums(factory);
122120

@@ -490,13 +488,7 @@ export class ModelClassVisitor {
490488
undefined,
491489
overrideType
492490
? f.createIdentifier(overrideType)
493-
: this.createTypePropertyAssignment(
494-
f,
495-
node.type,
496-
typeChecker,
497-
hostFilename,
498-
pluginOptions,
499-
),
491+
: this.getTypeUsingTypeChecker(f, node.type, typeChecker, hostFilename),
500492
);
501493

502494
const description = pluginOptions.introspectComments
@@ -517,49 +509,23 @@ export class ModelClassVisitor {
517509
return objectLiteral;
518510
}
519511

520-
private createTypePropertyAssignment(
512+
private getTypeUsingTypeChecker(
521513
f: ts.NodeFactory,
522514
node: ts.TypeNode,
523515
typeChecker: ts.TypeChecker,
524516
hostFilename: string,
525-
pluginOptions?: PluginOptions,
526517
) {
527-
if (node) {
528-
if (ts.isTypeLiteralNode(node)) {
529-
const propertyAssignments = Array.from(node.members || []).map(
530-
(member) => {
531-
const literalExpr = this.createDecoratorObjectLiteralExpr(
532-
f,
533-
member as ts.PropertySignature,
534-
typeChecker,
535-
undefined,
536-
hostFilename,
537-
pluginOptions,
538-
);
539-
return f.createPropertyAssignment(
540-
f.createIdentifier(member.name.getText()),
541-
literalExpr,
542-
);
543-
},
544-
);
545-
546-
return f.createParenthesizedExpression(
547-
f.createObjectLiteralExpression(propertyAssignments),
548-
);
549-
} else if (ts.isUnionTypeNode(node)) {
550-
const nullableType = findNullableTypeFromUnion(node, typeChecker);
551-
const remainingTypes = node.types.filter(
552-
(item) => item !== nullableType,
518+
if (node && ts.isUnionTypeNode(node)) {
519+
const nullableType = findNullableTypeFromUnion(node, typeChecker);
520+
const remainingTypes = node.types.filter((item) => item !== nullableType);
521+
522+
if (remainingTypes.length === 1) {
523+
return this.getTypeUsingTypeChecker(
524+
f,
525+
remainingTypes[0],
526+
typeChecker,
527+
hostFilename,
553528
);
554-
555-
if (remainingTypes.length === 1) {
556-
return this.createTypePropertyAssignment(
557-
f,
558-
remainingTypes[0],
559-
typeChecker,
560-
hostFilename,
561-
);
562-
}
563529
}
564530
}
565531

@@ -568,38 +534,31 @@ export class ModelClassVisitor {
568534
return undefined;
569535
}
570536

571-
let typeReference = getTypeReferenceAsString(type, typeChecker);
572-
if (!typeReference) {
537+
const _typeReference = getTypeReferenceAsString(type, typeChecker);
538+
539+
if (!_typeReference) {
573540
return undefined;
574541
}
575-
typeReference = replaceImportPath(typeReference, hostFilename);
576-
if (typeReference && typeReference.includes('require')) {
542+
543+
const { typeReference, importPath } = replaceImportPath(
544+
_typeReference,
545+
hostFilename,
546+
);
547+
548+
if (importPath) {
577549
// add top-level import to eagarly load class metadata
578-
const importPath = /\(\"([^)]).+(\")/.exec(typeReference)[0];
579-
if (importPath) {
580-
let importsToAdd = importsToAddPerFile.get(hostFilename);
581-
if (!importsToAdd) {
582-
importsToAdd = new Set();
583-
importsToAddPerFile.set(hostFilename, importsToAdd);
584-
}
585-
importsToAdd.add(importPath.slice(2, importPath.length - 1));
586-
}
550+
this.importsToAdd.add(importPath);
587551
}
588552

589553
return f.createIdentifier(typeReference);
590554
}
591555

592-
private createEagerImports(
593-
f: ts.NodeFactory,
594-
fileName: string,
595-
): ts.ImportEqualsDeclaration[] {
596-
const importsToAdd = importsToAddPerFile.get(fileName);
597-
598-
if (!importsToAdd) {
556+
private createEagerImports(f: ts.NodeFactory): ts.ImportEqualsDeclaration[] {
557+
if (!this.importsToAdd.size) {
599558
return [];
600559
}
601560

602-
return Array.from(importsToAdd).map((path, index) => {
561+
return Array.from(this.importsToAdd).map((path, index) => {
603562
return createImportEquals(f, 'eager_import_' + index, path);
604563
});
605564
}

tests/plugin/fixtures/create-cat-alt.dto.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ let CreateCatDto2 = class CreateCatDto2 {
6666
this.status = Status.ENABLED;
6767
}
6868
static _GRAPHQL_METADATA_FACTORY() {
69-
return { name: { type: () => String, description: "name description" }, age: { type: () => Number, description: "test on age" }, tags: { type: () => [String] }, status: { type: () => Status }, breed: { nullable: true, type: () => String }, nodes: { type: () => [Object] }, alias: { type: () => Object }, numberAlias: { type: () => Number }, union: { type: () => Object }, intersection: { type: () => Object }, optionalBoolean: { nullable: true, type: () => Boolean }, nested: { type: () => ({ first: { type: () => String }, second: { type: () => Number }, status: { type: () => Status }, tags: { type: () => [String] }, nodes: { type: () => [Object] }, alias: { type: () => Object }, numberAlias: { type: () => Number } }) }, tuple: { type: () => Object } };
69+
return { name: { type: () => String, description: "name description" }, age: { type: () => Number, description: "test on age" }, tags: { type: () => [String] }, status: { type: () => Status }, breed: { nullable: true, type: () => String }, nodes: { type: () => [Object] }, alias: { type: () => Object }, numberAlias: { type: () => Number }, union: { type: () => Object }, intersection: { type: () => Object }, optionalBoolean: { nullable: true, type: () => Boolean }, nested: { type: () => Object }, prop: { type: () => Object }, tuple: { type: () => Object } };
7070
}
7171
};
7272
CreateCatDto2 = __decorate([

tests/plugin/plugin-utils.spec.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { replaceImportPath } from '../../lib/plugin/utils/plugin-utils';
2+
3+
describe('plugin-utils', () => {
4+
describe('replaceImportPath', () => {
5+
it('should replace path to relative', () => {
6+
const actual = replaceImportPath(
7+
'import("/root/project/src/author.model").Author',
8+
'/root/project/src/post.model.ts',
9+
);
10+
11+
expect(actual).toStrictEqual({
12+
typeReference: 'require("./author.model").Author',
13+
importPath: './author.model',
14+
});
15+
});
16+
17+
it('should shorten path if it points to node_modules', () => {
18+
const actual = replaceImportPath(
19+
'import("/root/project/src/node_modules/dependency/author.model").Author',
20+
'/root/project/src/post.model.ts',
21+
);
22+
23+
expect(actual).toStrictEqual({
24+
typeReference: 'require("dependency/author.model").Author',
25+
importPath: 'dependency/author.model',
26+
});
27+
});
28+
29+
it('should shorten path if it points to node_modules/@types', () => {
30+
const actual = replaceImportPath(
31+
'import("/root/project/src/node_modules/@types/dependency/author.model").Author',
32+
'/root/project/src/post.model.ts',
33+
);
34+
35+
expect(actual).toStrictEqual({
36+
typeReference: 'require("dependency/author.model").Author',
37+
importPath: 'dependency/author.model',
38+
});
39+
});
40+
41+
it('should shorten path if it points to dependency index file', () => {
42+
const actual = replaceImportPath(
43+
'import("/root/project/src/node_modules/dependency/index").Author',
44+
'/root/project/src/post.model.ts',
45+
);
46+
47+
expect(actual).toStrictEqual({
48+
typeReference: 'require("dependency").Author',
49+
importPath: 'dependency',
50+
});
51+
});
52+
});
53+
});

0 commit comments

Comments
 (0)