From 14cb89adc88e0b326a7581f31e4b841f90c31398 Mon Sep 17 00:00:00 2001 From: Shubh-Raj Date: Thu, 22 Jan 2026 16:26:56 +0530 Subject: [PATCH] fix(typescript): propagate optional modifier for scalar fields When generating TypeScript code, optional scalar fields were losing their optional modifier. This fix propagates the optional property from the parent field via the forceOptional parameter, avoiding mutation of the shared scalar descriptor. Signed-off-by: Shubh-Raj --- .../fromcto/typescript/typescriptvisitor.js | 7 ++-- package-lock.json | 23 ++++++++----- .../fromcto/typescript/typescriptvisitor.js | 33 +++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/lib/codegen/fromcto/typescript/typescriptvisitor.js b/lib/codegen/fromcto/typescript/typescriptvisitor.js index 5ffd94d4..d6a34c3f 100644 --- a/lib/codegen/fromcto/typescript/typescriptvisitor.js +++ b/lib/codegen/fromcto/typescript/typescriptvisitor.js @@ -47,7 +47,10 @@ class TypescriptVisitor { } else if (thing.isMapDeclaration?.()) { return this.visitMapDeclaration(thing, parameters); } else if (thing.isTypeScalar?.()) { - return this.visitField(thing.getScalarField(), parameters); + // Propagate optional modifier from the field to visitField via parameters + // Avoid mutating the shared scalar descriptor + const scalarField = thing.getScalarField(); + return this.visitField(scalarField, { ...parameters, forceOptional: thing.isOptional() }); } else if (thing.isField?.()) { return this.visitField(thing, parameters); } else if (thing.isRelationship?.()) { @@ -247,7 +250,7 @@ class TypescriptVisitor { array = '[]'; } - if (field.isOptional()) { + if (field.isOptional() || parameters.forceOptional) { optional = '?'; } const isEnumRef = field.isPrimitive() ? false diff --git a/package-lock.json b/package-lock.json index 52daff30..2bee8a7d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -231,6 +231,7 @@ "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.26.0.tgz", "integrity": "sha512-i1SLeK+DzNnQ3LL/CswPCa/E5u4lh1k6IAEphON8F+cXt0t9euTshDru0q7/IqMa1PMPz5RnHuHscF8/ZJsStg==", "dev": true, + "peer": true, "dependencies": { "@ampproject/remapping": "^2.2.0", "@babel/code-frame": "^7.26.0", @@ -2923,6 +2924,7 @@ "resolved": "https://registry.npmjs.org/@types/markdown-it/-/markdown-it-14.1.2.tgz", "integrity": "sha512-promo4eFwuiW+TfGxhi+0x3czqTYJkG8qB17ZUJiVF10Xm7NLVRSLUsfRTU/6h1e24VvRnXCx+hG7li58lkzog==", "dev": true, + "peer": true, "dependencies": { "@types/linkify-it": "^5", "@types/mdurl": "^2" @@ -3218,6 +3220,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.14.0.tgz", "integrity": "sha512-cl669nCJTZBsL97OF4kUQm5g5hC2uihk0NxY3WENAC0TYdILVkAyHymAntgxGkl7K+t0cXIrH5siy5S4XkFycA==", "dev": true, + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -3403,8 +3406,7 @@ "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==", - "dev": true, - "peer": true + "dev": true }, "node_modules/available-typed-arrays": { "version": "1.0.7", @@ -3772,6 +3774,7 @@ "url": "https://github.com/sponsors/ai" } ], + "peer": true, "dependencies": { "caniuse-lite": "^1.0.30001669", "electron-to-chromium": "^1.5.41", @@ -3970,6 +3973,7 @@ "resolved": "https://registry.npmjs.org/chai/-/chai-4.3.6.tgz", "integrity": "sha512-bbcp3YfHCUzMOvKqsztczerVgBKSsEijCySNlHHbX3VG1nskvqjz5Rfso1gGwD6w6oOV3eI60pKuMOV5MV7p3Q==", "dev": true, + "peer": true, "dependencies": { "assertion-error": "^1.1.0", "check-error": "^1.0.2", @@ -4168,7 +4172,6 @@ "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", "integrity": "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==", "dev": true, - "peer": true, "dependencies": { "delayed-stream": "~1.0.0" }, @@ -4463,7 +4466,6 @@ "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", "integrity": "sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==", "dev": true, - "peer": true, "engines": { "node": ">=0.4.0" } @@ -4720,7 +4722,6 @@ "integrity": "sha512-j6vWzfrGVfyXxge+O0x5sh6cvxAog0a/4Rdd2K36zCMV5eJ+/+tOAngRO8cODMNWbVRdVlmGZQL2YS3yR8bIUA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "es-errors": "^1.3.0", "get-intrinsic": "^1.2.6", @@ -5268,7 +5269,6 @@ "url": "https://github.com/sponsors/RubenVerborgh" } ], - "peer": true, "engines": { "node": ">=4.0" }, @@ -5306,7 +5306,6 @@ "integrity": "sha512-KrGhL9Q4zjj0kiUt5OO4Mr/A/jlI2jDYs5eHBpYHPcBEVSiipAvn2Ko2HnPe20rmcuuvMHNdZFp+4IlGTMF0Ow==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", @@ -6798,6 +6797,7 @@ "resolved": "https://registry.npmjs.org/markdown-it/-/markdown-it-14.1.0.tgz", "integrity": "sha512-a54IwgWPaeBCAAsv13YgmALOF1elABB08FxO9i+r4VFk5Vl4pKokRPeX8u5TCgSsPi6ec1otfLjdOpVcgbpshg==", "dev": true, + "peer": true, "dependencies": { "argparse": "^2.0.1", "entities": "^4.4.0", @@ -8219,8 +8219,7 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "dev": true, - "peer": true + "dev": true }, "node_modules/public-encrypt": { "version": "4.0.3", @@ -8684,6 +8683,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "dev": true, + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -8845,6 +8845,7 @@ "integrity": "sha512-wVMT1jvgyLroQu+tWTa2XJu2g5SoVaEBhdwGPuzmWlRvZNSG+pcEb8HEmsxTdoPjUL58pJFQ3+oFRWrhC4JKHw==", "deprecated": "16.1.1", "dev": true, + "peer": true, "dependencies": { "@sinonjs/commons": "^1.8.3", "@sinonjs/fake-timers": "^8.1.0", @@ -9332,6 +9333,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "dev": true, + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -9913,6 +9915,7 @@ "resolved": "https://registry.npmjs.org/webpack/-/webpack-5.94.0.tgz", "integrity": "sha512-KcsGn50VT+06JH/iunZJedYGUJS5FGjow8wb9c0v5n1Om8O1g4L6LjtfxwlXIATopoQu+vOXXa7gYisWxCoPyg==", "dev": true, + "peer": true, "dependencies": { "@types/estree": "^1.0.5", "@webassemblyjs/ast": "^1.12.1", @@ -9959,6 +9962,7 @@ "resolved": "https://registry.npmjs.org/webpack-cli/-/webpack-cli-4.9.1.tgz", "integrity": "sha512-JYRFVuyFpzDxMDB+v/nanUdQYcZtqFPGzmlW4s+UkPMFhSpfRNmf1z4AwYcHJVdvEFAM7FFCQdNTpsBYhDLusQ==", "dev": true, + "peer": true, "dependencies": { "@discoveryjs/json-ext": "^0.5.0", "@webpack-cli/configtest": "^1.1.0", @@ -10034,6 +10038,7 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz", "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==", "dev": true, + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", diff --git a/test/codegen/fromcto/typescript/typescriptvisitor.js b/test/codegen/fromcto/typescript/typescriptvisitor.js index 39fe80b9..e42a27d0 100644 --- a/test/codegen/fromcto/typescript/typescriptvisitor.js +++ b/test/codegen/fromcto/typescript/typescriptvisitor.js @@ -135,6 +135,39 @@ describe('TypescriptVisitor', function () { mockSpecialVisit.calledWith(thing, param).should.be.ok; }); + it('should propagate optional modifier for scalar fields via parameters', () => { + // Create a mock scalar field + let mockScalarField = sinon.createStubInstance(Field); + mockScalarField.isOptional.returns(false); // Scalar field itself is not optional + + // Create a thing that is a scalar type field + let thing = { + isModelManager: () => false, + isModelFile: () => false, + isEnum: () => false, + isClassDeclaration: () => false, + isMapDeclaration: () => false, + isTypeScalar: () => true, + isField: () => true, + isRelationship: () => false, + isEnumValue: () => false, + isOptional: () => true, // The parent field is optional + getScalarField: () => mockScalarField + }; + + let mockSpecialVisit = sinon.stub(typescriptVisitor, 'visitField'); + mockSpecialVisit.returns('Duck'); + + typescriptVisitor.visit(thing, param); + + // Verify that visitField was called with the scalar field + mockSpecialVisit.calledOnce.should.be.ok; + // Verify that forceOptional was passed via parameters (not by mutating the scalar field) + const callArgs = mockSpecialVisit.getCall(0).args; + callArgs[0].should.equal(mockScalarField); + callArgs[1].forceOptional.should.equal(true); + }); + it('should throw an error when an unrecognised type is supplied', () => { let thing = 'Something of unrecognised type';