From a161786c6b966171ad189c43075ca1095f467b40 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Thu, 21 May 2026 19:59:09 +0800 Subject: [PATCH] fix(api-compat): allow optional property additions to inline object literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit areInterfaceSignaturesCompatible compared each property's type as a string, then only re-checked via isUnionTypeWidening when the strings differed. Adding an optional property to an inline object literal (e.g. mastra?: boolean to InstrumentationConfig.integrations) flips the string equality, isn't a union widening, and was incorrectly classified as a breaking change — even though it's structurally backward-compatible TypeScript. Fix: delegate to the existing areObjectTypeDefinitionsCompatible helper when both sides are inline object literals, mirroring the pattern already in areTypeAliasSignaturesCompatible. Also apply the same delegation inside areObjectTypeDefinitionsCompatible itself so nested object literals get the same treatment. Lifted the inline isObjectType helper to a module-scope isObjectLiteralType so both call sites share it. Adds 4 regression tests: - optional property added to inline object literal (the InstrumentationConfig case) - required property added → still rejected - property removed → still rejected - optional property added to deeply nested inline object literal Co-Authored-By: Claude Opus 4.7 (1M context) --- .../api-compatibility.test.ts | 69 ++++++++++++++++--- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/js/tests/api-compatibility/api-compatibility.test.ts b/js/tests/api-compatibility/api-compatibility.test.ts index 2fa625c1c..3528f49ee 100644 --- a/js/tests/api-compatibility/api-compatibility.test.ts +++ b/js/tests/api-compatibility/api-compatibility.test.ts @@ -785,6 +785,11 @@ function normalizeTypeReference(type: string): string { return type; } +function isObjectLiteralType(def: string): boolean { + const trimmed = def.trim(); + return trimmed.startsWith("{") && trimmed.endsWith("}"); +} + function parseObjectTypeProps( def: string, ): Map { @@ -877,7 +882,15 @@ function areObjectTypeDefinitionsCompatible( const newTypeNorm = normalizeType(newProp.type); if (oldTypeNorm !== newTypeNorm) { - if (!isUnionTypeWidening(oldTypeNorm, newTypeNorm)) { + if (isUnionTypeWidening(oldTypeNorm, newTypeNorm)) { + // accepted + } else if ( + isObjectLiteralType(oldProp.type) && + isObjectLiteralType(newProp.type) && + areObjectTypeDefinitionsCompatible(oldProp.type, newProp.type) + ) { + // accepted: nested object literal is structurally compatible + } else { return false; } } @@ -1040,10 +1053,7 @@ function areTypeAliasSignaturesCompatible( return true; } - // Check if it's an object type - const isObjectType = (def: string) => def.trim().startsWith("{"); - - if (isObjectType(oldDef) && isObjectType(newDef)) { + if (isObjectLiteralType(oldDef) && isObjectLiteralType(newDef)) { return areObjectTypeDefinitionsCompatible(oldDef, newDef); } @@ -1089,8 +1099,8 @@ function areTypeAliasSignaturesCompatible( } if ( - isObjectType(oldPart) && - isObjectType(newPart) && + isObjectLiteralType(oldPart) && + isObjectLiteralType(newPart) && areObjectTypeDefinitionsCompatible(oldPart, newPart) ) { usedNewIndices.add(index); @@ -1337,8 +1347,16 @@ function areInterfaceSignaturesCompatible( const newTypeNorm = normalizeType(newField.type); if (oldTypeNorm !== newTypeNorm) { - // Check if it's a union type widening (backwards compatible) - if (!isUnionTypeWidening(oldTypeNorm, newTypeNorm)) { + if (isUnionTypeWidening(oldTypeNorm, newTypeNorm)) { + // accepted: union type widened (e.g. `T` → `T | U`) + } else if ( + isObjectLiteralType(oldField.type) && + isObjectLiteralType(newField.type) && + areObjectTypeDefinitionsCompatible(oldField.type, newField.type) + ) { + // accepted: inline object literal field is structurally compatible + // (e.g. an optional property added to `integrations`) + } else { // Field type changed in an incompatible way - breaking change return false; } @@ -2243,6 +2261,39 @@ describe("areInterfaceSignaturesCompatible", () => { const result = areInterfaceSignaturesCompatible(oldInterface, newInterface); expect(result).toBe(false); }); + + test("should allow adding optional property to an inline object literal field", () => { + // Real-world case: adding `mastra?: boolean` to InstrumentationConfig.integrations + const oldInterface = `export interface InstrumentationConfig { integrations?: { openai?: boolean; openaiCodexSDK?: boolean; }; }`; + const newInterface = `export interface InstrumentationConfig { integrations?: { openai?: boolean; openaiCodexSDK?: boolean; mastra?: boolean; }; }`; + + const result = areInterfaceSignaturesCompatible(oldInterface, newInterface); + expect(result).toBe(true); + }); + + test("should reject adding required property to an inline object literal field", () => { + const oldInterface = `export interface Config { options: { foo?: string; }; }`; + const newInterface = `export interface Config { options: { foo?: string; bar: number; }; }`; + + const result = areInterfaceSignaturesCompatible(oldInterface, newInterface); + expect(result).toBe(false); + }); + + test("should reject removing property from an inline object literal field", () => { + const oldInterface = `export interface Config { options: { foo?: string; bar?: number; }; }`; + const newInterface = `export interface Config { options: { foo?: string; }; }`; + + const result = areInterfaceSignaturesCompatible(oldInterface, newInterface); + expect(result).toBe(false); + }); + + test("should allow adding optional property to a deeply nested inline object literal field", () => { + const oldInterface = `export interface Config { outer: { inner: { a?: string; }; }; }`; + const newInterface = `export interface Config { outer: { inner: { a?: string; b?: number; }; }; }`; + + const result = areInterfaceSignaturesCompatible(oldInterface, newInterface); + expect(result).toBe(true); + }); }); describe("areClassSignaturesCompatible", () => {