diff --git a/lib/codegen/fromcto/typescript/typescriptvisitor.js b/lib/codegen/fromcto/typescript/typescriptvisitor.js index ea36feb..e98a7f0 100644 --- a/lib/codegen/fromcto/typescript/typescriptvisitor.js +++ b/lib/codegen/fromcto/typescript/typescriptvisitor.js @@ -144,29 +144,21 @@ class TypescriptVisitor { const typeName = ModelUtil.getShortName(property.getFullyQualifiedTypeName()); addImport(typeNamespace, property.isTypeEnum?.() || property.isTypeScalar?.() ? typeName : `I${typeName}`); - } - }); - const subclasses = declaration.getDirectSubclasses(); - if (subclasses && subclasses.length > 0) { - parameters.fileWriter.writeLine(0, '\n// Warning: Beware of circular dependencies when modifying these imports'); - - // Group subclasses by namespace - const namespaceBuckets = {}; - subclasses.map(subclass => { - const bucket = namespaceBuckets[subclass.getNamespace()]; - if (bucket){ - bucket.push(subclass); - } else { - namespaceBuckets[subclass.getNamespace()] = [subclass]; + // When flattenSubclassesToUnion is set, also import the union type + if (parameters.flattenSubclassesToUnion && !property.isTypeEnum?.() && !property.isTypeScalar?.()) { + const propDecl = modelFile.getModelManager().getType(property.getFullyQualifiedTypeName()); + const subclasses = propDecl?.getDirectSubclasses?.(); + if (subclasses) { + const sameNsSubs = subclasses.filter(sc => + !sc.isEnum() && sc.getNamespace() === propDecl.getNamespace()); + if (sameNsSubs.length > 1) { + addImport(typeNamespace, `${typeName}Union`); + } + } } - }); - Object.entries(namespaceBuckets) - .filter(([namespace]) => namespace !== modelFile.getNamespace()) // Skip own namespace - .map(([namespace, bucket]) => { - parameters.fileWriter.writeLine(0, `import type {\n\t${bucket.map(subclass => subclass.isEnum() ? subclass.getName() : `I${subclass.getName()}`).join(',\n\t') }\n} from './${namespace}';`); - }); - } + } + }); } }); @@ -245,11 +237,15 @@ class TypescriptVisitor { parameters.fileWriter.writeLine(0, '}\n'); - // If there exists direct subclasses for this declaration then generate a union for it + // Generate a union alias only when there are multiple same-namespace subclasses const subclasses = classDeclaration.getDirectSubclasses(); - if (subclasses && subclasses.length > 0) { - parameters.fileWriter.writeLine(0, 'export type ' + classDeclaration.getName() + - 'Union = ' + subclasses.filter(declaration => !declaration.isEnum()).map(subclass => `I${subclass.getName()}`).join(' | \n') + ';\n'); + if (subclasses) { + const sameNsSubclasses = subclasses + .filter(sc => !sc.isEnum() && sc.getNamespace() === classDeclaration.getNamespace()); + if (sameNsSubclasses.length > 1) { + parameters.fileWriter.writeLine(0, 'export type ' + classDeclaration.getName() + + 'Union = ' + sameNsSubclasses.map(subclass => `I${subclass.getName()}`).join(' | \n') + ';\n'); + } } return null; } @@ -301,12 +297,17 @@ class TypescriptVisitor { let tsType = this.toTsType(field.getType(), !isEnumRef && !hasUnion && !isMapRef, hasUnion); - // If there exists direct subclasses for this field's declaration then use the union type instead - if (!!parameters.flattenSubclassesToUnion & !field.isPrimitive()) { - const subclasses = field.getParent().getModelFile().getModelManager().getType(field.getFullyQualifiedTypeName()).getDirectSubclasses(); - if (subclasses && subclasses.length > 0) { - const useUnion = !(isEnumRef || isMapRef); - tsType = this.toTsType(field.getType(), !useUnion, useUnion); + // Use the union type only when there are multiple same-namespace subclasses + if (!!parameters.flattenSubclassesToUnion && !field.isPrimitive()) { + const fieldDecl = field.getParent().getModelFile().getModelManager().getType(field.getFullyQualifiedTypeName()); + const subclasses = fieldDecl.getDirectSubclasses(); + if (subclasses) { + const sameNsSubclasses = subclasses + .filter(sc => !sc.isEnum() && sc.getNamespace() === fieldDecl.getNamespace()); + if (sameNsSubclasses.length > 1) { + const useUnion = !(isEnumRef || isMapRef); + tsType = this.toTsType(field.getType(), !useUnion, useUnion); + } } } diff --git a/test/codegen/__snapshots__/codegen.js.snap b/test/codegen/__snapshots__/codegen.js.snap index 8c47103..b0aefe7 100644 --- a/test/codegen/__snapshots__/codegen.js.snap +++ b/test/codegen/__snapshots__/codegen.js.snap @@ -6753,19 +6753,12 @@ exports[`codegen #formats check we can convert all formats from namespace versio // Generated code for namespace: concerto.decorator@1.0.0 // imports - -// Warning: Beware of circular dependencies when modifying these imports -import type { - Ipii -} from './org.acme.hr@1.0.0'; import {IConcept} from './concerto@1.0.0'; // interfaces export interface IDecorator extends IConcept { } -export type DecoratorUnion = Ipii; - export interface IDotNetNamespace extends IDecorator { namespace: string; } @@ -6782,77 +6775,27 @@ exports[`codegen #formats check we can convert all formats from namespace versio // imports -// Warning: Beware of circular dependencies when modifying these imports -import type { - ICategory, - State, - TShirtSizeType, - IAddress, - Level -} from './org.acme.hr.base@1.0.0'; -import type { - ICategory, - IInfo, - ICompany, - Department, - LaptopMake -} from './org.acme.hr@1.0.0'; - -// Warning: Beware of circular dependencies when modifying these imports -import type { - IEquipment -} from './org.acme.hr@1.0.0'; - -// Warning: Beware of circular dependencies when modifying these imports -import type { - IPerson -} from './org.acme.hr@1.0.0'; - -// Warning: Beware of circular dependencies when modifying these imports -import type { - IChangeOfAddress -} from './org.acme.hr@1.0.0'; - -// Warning: Beware of circular dependencies when modifying these imports -import type { - ICompanyEvent -} from './org.acme.hr@1.0.0'; - // interfaces export interface IConcept { $class: string; } -export type ConceptUnion = ICategory | -IAddress | -ICategory | -IInfo | -ICompany; - export interface IAsset extends IConcept { $identifier: string; } -export type AssetUnion = IEquipment; - export interface IParticipant extends IConcept { $identifier: string; } -export type ParticipantUnion = IPerson; - export interface ITransaction extends IConcept { $timestamp: Date; } -export type TransactionUnion = IChangeOfAddress; - export interface IEvent extends IConcept { $timestamp: Date; } -export type EventUnion = ICompanyEvent; - ", } `; @@ -6864,16 +6807,12 @@ exports[`codegen #formats check we can convert all formats from namespace versio // Generated code for namespace: org.acme.hr.base@1.0.0 // imports - -// Warning: Beware of circular dependencies when modifying these imports import {IConcept} from './concerto@1.0.0'; // interfaces export interface ICategory extends IConcept { } -export type CategoryUnion = IGeneralCategory; - export interface IGeneralCategory extends ICategory { } @@ -6920,14 +6859,6 @@ exports[`codegen #formats check we can convert all formats from namespace versio // Generated code for namespace: org.acme.hr@1.0.0 // imports - -// Warning: Beware of circular dependencies when modifying these imports - -// Warning: Beware of circular dependencies when modifying these imports - -// Warning: Beware of circular dependencies when modifying these imports - -// Warning: Beware of circular dependencies when modifying these imports import {Time,SSN,IAddress,IEmployeeTShirtSizes} from './org.acme.hr.base@1.0.0'; import {IDecorator} from './concerto.decorator@1.0.0'; import {IConcept,IAsset,IParticipant,IEvent,ITransaction} from './concerto@1.0.0'; @@ -6980,8 +6911,6 @@ export interface IEquipment extends IAsset { serialNumber: string; } -export type EquipmentUnion = ILaptop; - export enum LaptopMake { Apple = 'Apple', Microsoft = 'Microsoft', @@ -7017,8 +6946,6 @@ export interface IEmployee extends IPerson { manager?: IManager; } -export type EmployeeUnion = IManager; - export interface IContractor extends IPerson { company: ICompany; manager?: IManager; @@ -7031,8 +6958,6 @@ export interface IManager extends IEmployee { export interface ICompanyEvent extends IEvent { } -export type CompanyEventUnion = IOnboarded; - export interface IOnboarded extends ICompanyEvent { employee: IEmployee; } diff --git a/test/codegen/fromcto/typescript/typescriptvisitor.js b/test/codegen/fromcto/typescript/typescriptvisitor.js index 6eca3a0..84eda8f 100644 --- a/test/codegen/fromcto/typescript/typescriptvisitor.js +++ b/test/codegen/fromcto/typescript/typescriptvisitor.js @@ -352,7 +352,7 @@ describe('TypescriptVisitor', function () { acceptSpy.withArgs(typescriptVisitor, param).calledTwice.should.be.ok; }); - it('should write lines for the imports of direct subclasses that are not in the same namespace', () => { + it('should not write cross-namespace subclass imports since unions are scoped to same namespace', () => { let acceptSpy = sinon.spy(); let mockSubclassDeclaration1 = sinon.createStubInstance(ClassDeclaration); @@ -397,13 +397,11 @@ describe('TypescriptVisitor', function () { typescriptVisitor.visitModelFile(mockModelFile, param); param.fileWriter.openFile.withArgs('org.acme.ts').calledOnce.should.be.ok; - param.fileWriter.writeLine.callCount.should.deep.equal(6); + param.fileWriter.writeLine.callCount.should.deep.equal(4); param.fileWriter.writeLine.getCall(0).args.should.deep.equal([0, '/* eslint-disable @typescript-eslint/no-empty-interface */']); param.fileWriter.writeLine.getCall(1).args.should.deep.equal([0, '// Generated code for namespace: org.acme']); param.fileWriter.writeLine.getCall(2).args.should.deep.equal([0, '\n// imports']); - param.fileWriter.writeLine.getCall(3).args.should.deep.equal([0, '\n// Warning: Beware of circular dependencies when modifying these imports']); - param.fileWriter.writeLine.getCall(4).args.should.deep.equal([0, 'import type {\n\tIImportedDirectSubclass,\n\tIImportedDirectSubclass2\n} from \'./org.acme.subclasses\';']); - param.fileWriter.writeLine.getCall(5).args.should.deep.equal([0, '\n// interfaces']); + param.fileWriter.writeLine.getCall(3).args.should.deep.equal([0, '\n// interfaces']); param.fileWriter.closeFile.calledOnce.should.be.ok; acceptSpy.withArgs(typescriptVisitor, param).calledOnce.should.be.ok; @@ -631,7 +629,7 @@ describe('TypescriptVisitor', function () { param.fileWriter.writeLine.getCall(1).args.should.deep.equal([0, '}\n']); }); - it('should create a union given a class that has dependencies but no super class', () => { + it('should not create a union given a class with only one same-namespace subclass', () => { let acceptSpy = sinon.spy(); let mockChildClassDeclaration = sinon.createStubInstance(ClassDeclaration); @@ -643,6 +641,7 @@ describe('TypescriptVisitor', function () { accept: acceptSpy }]); mockChildClassDeclaration.getName.returns('Child'); + mockChildClassDeclaration.getNamespace.returns('org.acme'); mockChildClassDeclaration.isAbstract.returns(false); mockChildClassDeclaration.getSuperType.returns('Parent'); @@ -655,17 +654,52 @@ describe('TypescriptVisitor', function () { accept: acceptSpy }]); mockClassDeclaration.getName.returns('Parent'); + mockClassDeclaration.getNamespace.returns('org.acme'); mockClassDeclaration.isAbstract.returns(true); mockClassDeclaration.getSuperType.returns(null); mockClassDeclaration.getDirectSubclasses.returns([mockChildClassDeclaration]); typescriptVisitor.visitClassDeclaration(mockClassDeclaration, param); + param.fileWriter.writeLine.callCount.should.deep.equal(3); + param.fileWriter.writeLine.getCall(0).args.should.deep.equal([0, 'export interface IParent {']); + param.fileWriter.writeLine.getCall(1).args.should.deep.equal([1, '$class: string;']); + param.fileWriter.writeLine.getCall(2).args.should.deep.equal([0, '}\n']); + }); + + it('should create a union given a class with multiple same-namespace subclasses', () => { + let acceptSpy = sinon.spy(); + + let mockChildClassDeclaration1 = sinon.createStubInstance(ClassDeclaration); + mockChildClassDeclaration1.isClassDeclaration.returns(true); + mockChildClassDeclaration1.getName.returns('Child1'); + mockChildClassDeclaration1.getNamespace.returns('org.acme'); + mockChildClassDeclaration1.isAbstract.returns(false); + + let mockChildClassDeclaration2 = sinon.createStubInstance(ClassDeclaration); + mockChildClassDeclaration2.isClassDeclaration.returns(true); + mockChildClassDeclaration2.getName.returns('Child2'); + mockChildClassDeclaration2.getNamespace.returns('org.acme'); + mockChildClassDeclaration2.isAbstract.returns(false); + + let mockClassDeclaration = sinon.createStubInstance(ClassDeclaration); + mockClassDeclaration.isClassDeclaration.returns(true); + mockClassDeclaration.getOwnProperties.returns([{ + accept: acceptSpy + }]); + mockClassDeclaration.getName.returns('Parent'); + mockClassDeclaration.getNamespace.returns('org.acme'); + mockClassDeclaration.isAbstract.returns(true); + mockClassDeclaration.getSuperType.returns(null); + mockClassDeclaration.getDirectSubclasses.returns([mockChildClassDeclaration1, mockChildClassDeclaration2]); + + typescriptVisitor.visitClassDeclaration(mockClassDeclaration, param); + param.fileWriter.writeLine.callCount.should.deep.equal(4); param.fileWriter.writeLine.getCall(0).args.should.deep.equal([0, 'export interface IParent {']); param.fileWriter.writeLine.getCall(1).args.should.deep.equal([1, '$class: string;']); param.fileWriter.writeLine.getCall(2).args.should.deep.equal([0, '}\n']); - param.fileWriter.writeLine.getCall(3).args.should.deep.equal([0, 'export type ParentUnion = IChild;\n']); + param.fileWriter.writeLine.getCall(3).args.should.deep.equal([0, 'export type ParentUnion = IChild1 | \nIChild2;\n']); }); it('should not create a union if a class has no sub-classes', () => { @@ -832,7 +866,7 @@ describe('TypescriptVisitor', function () { param.fileWriter.writeLine.withArgs(1, 'literalTest = EnumType.MyEnumValue;').calledOnce.should.be.ok; }); - it('should write a line for field name using a union type when the flattenSubclassesToUnion parameter is set', () => { + it('should write a line for field name using a union type when the flattenSubclassesToUnion parameter is set and there are multiple same-namespace subclasses', () => { const mockField = sinon.createStubInstance(Field); mockField.isPrimitive.returns(false); mockField.getName.returns('flattenSubclassesTest'); @@ -842,7 +876,16 @@ describe('TypescriptVisitor', function () { const mockModelManager = sinon.createStubInstance(ModelManager); const mockModelFile = sinon.createStubInstance(ModelFile); const mockClassDeclaration = sinon.createStubInstance(ClassDeclaration); - mockClassDeclaration.getDirectSubclasses.returns(['blah']); // Not valid, but sufficient for this test + + const mockSubclass1 = sinon.createStubInstance(ClassDeclaration); + mockSubclass1.getNamespace.returns('org.acme'); + mockSubclass1.isEnum.returns(false); + const mockSubclass2 = sinon.createStubInstance(ClassDeclaration); + mockSubclass2.getNamespace.returns('org.acme'); + mockSubclass2.isEnum.returns(false); + + mockClassDeclaration.getDirectSubclasses.returns([mockSubclass1, mockSubclass2]); + mockClassDeclaration.getNamespace.returns('org.acme'); mockModelManager.getType.returns(mockClassDeclaration); mockClassDeclaration.isEnum.returns(false); @@ -853,6 +896,34 @@ describe('TypescriptVisitor', function () { param.fileWriter.writeLine.withArgs(1, 'flattenSubclassesTest: AnimalUnion;').calledOnce.should.be.ok; }); + + it('should not use union type when flattenSubclassesToUnion is set but there is only one same-namespace subclass', () => { + const mockField = sinon.createStubInstance(Field); + mockField.isPrimitive.returns(false); + mockField.getName.returns('singleSubclassTest'); + mockField.getType.returns('Animal'); + mockField.getDecorators.returns([]); + + const mockModelManager = sinon.createStubInstance(ModelManager); + const mockModelFile = sinon.createStubInstance(ModelFile); + const mockClassDeclaration = sinon.createStubInstance(ClassDeclaration); + + const mockSubclass1 = sinon.createStubInstance(ClassDeclaration); + mockSubclass1.getNamespace.returns('org.acme'); + mockSubclass1.isEnum.returns(false); + + mockClassDeclaration.getDirectSubclasses.returns([mockSubclass1]); + mockClassDeclaration.getNamespace.returns('org.acme'); + + mockModelManager.getType.returns(mockClassDeclaration); + mockClassDeclaration.isEnum.returns(false); + mockModelFile.getModelManager.returns(mockModelManager); + mockClassDeclaration.getModelFile.returns(mockModelFile); + mockField.getParent.returns(mockClassDeclaration); + typescriptVisitor.visitField(mockField, { ...param, flattenSubclassesToUnion: true }); + + param.fileWriter.writeLine.withArgs(1, 'singleSubclassTest: IAnimal;').calledOnce.should.be.ok; + }); }); describe('visitEnumValueDeclaration', () => {