fix(typescript): propagate optional modifier for scalar fields

[Shubh-Raj]

Closes #194

This PR fixes the bug where optional scalar fields lose their optional modifier (?) when generating TypeScript code.

Changes

• Modified visit() in lib/codegen/fromcto/typescript/typescriptvisitor.js to pass forceOptional via parameters when handling scalar fields, avoiding mutation of the shared scalar descriptor
• Updated visitField() to honor parameters.forceOptional when determining if a field should have the ? modifier
• Added unit test to verify that optional scalar fields correctly propagate their optional status via parameters

Flags

• None - this is a targeted bug fix with no breaking changes
• All 567 existing tests pass with 99%+ coverage

Screenshots or Video

Before fix: nickname: string; (missing ? for optional scalar)

After fix: nickname?: string; (correctly includes ?)

Related Issues

• Issue fix(typescript): optional scalar fields lose their optional modifier #194
• Related TODO in accordproject/concerto at packages/concertino/scripts/generateTypes.js:119

Author Checklist

• Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
• Vital features and changes captured in unit and/or integration tests
• Commits messages follow AP format
• Extend the documentation, if necessary
• Merging to main from fork:fix/scalar-optional-typescript

[Shubh-Raj]

@ekarademir @mttrbrts @dselman resolved the merge conflicts. Requesting a review.

[Copilot]

Pull request overview

This PR fixes a bug (#194) where optional scalar fields lose their ? (optional) modifier when generating TypeScript code from Concerto models. When a field uses a scalar type (e.g., o MyString nickname optional), the TypeScript visitor dispatches to visitField with the scalar field descriptor, which doesn't carry the optionality of the parent field. The fix passes forceOptional via a spread of the parameters object instead of mutating the shared scalar descriptor.

Changes:

• Modified the isTypeScalar branch in visit() to pass forceOptional: thing.isOptional() via a spread of the parameters object, avoiding mutation of the shared scalar descriptor.
• Updated visitField() to honor parameters.forceOptional when determining whether to emit the ? optional modifier.
• Added a unit test verifying that forceOptional is correctly passed through to visitField when visiting an optional scalar field.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/codegen/fromcto/typescript/typescriptvisitor.js	Core fix: propagates optional modifier via parameters spread and checks forceOptional in visitField
test/codegen/fromcto/typescript/typescriptvisitor.js	New unit test for the visit() dispatch to visitField with forceOptional in parameters

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new test verifies that visitField is called with forceOptional: true in the parameters, but it does not verify the actual rendering output. The test stubs out visitField entirely, so it never exercises the updated condition field.isOptional() || parameters.forceOptional at line 253 of the source file. A test in the visitField describe block that passes { forceOptional: true } as parameters (with a field where isOptional() returns false) and asserts that the output contains ? would provide meaningful end-to-end coverage of the fix.

Suggested change

	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);
	// Ensure the visitor has a FileWriter so that visitField can render output
	param.fileWriter = mockFileWriter;
	// Spy on visitField to exercise the real implementation while inspecting parameters
	let visitFieldSpy = sinon.spy(typescriptVisitor, 'visitField');
	typescriptVisitor.visit(thing, param);
	// Verify that visitField was called with the scalar field
	visitFieldSpy.calledOnce.should.be.ok;
	// Verify that forceOptional was passed via parameters (not by mutating the scalar field)
	const callArgs = visitFieldSpy.getCall(0).args;
	callArgs[0].should.equal(mockScalarField);
	callArgs[1].forceOptional.should.equal(true);
	// Verify that the rendered output reflects the optional modifier (contains '?')
	mockFileWriter.writeLine.called.should.be.ok;
	const renderedLine = mockFileWriter.writeLine.getCall(0).args[0];
	renderedLine.should.contain('?');

[Copilot]

This test does not assert the return value of typescriptVisitor.visit(thing, param), unlike every other test in the visit describe block, all of which assert .should.deep.equal('Duck'). The return value of the scalar path should also be verified.

Suggested change

	typescriptVisitor.visit(thing, param);
	const result = typescriptVisitor.visit(thing, param);
	result.should.deep.equal('Duck');