diff --git a/xlr/converters/src/__tests__/__snapshots__/player.test.ts.snap b/xlr/converters/src/__tests__/__snapshots__/player.test.ts.snap index cfb0c488..b65998e8 100644 --- a/xlr/converters/src/__tests__/__snapshots__/player.test.ts.snap +++ b/xlr/converters/src/__tests__/__snapshots__/player.test.ts.snap @@ -1831,7 +1831,9 @@ If the expression is a composite, the last expression executed is the return val "name": "NavigationFlowState", "or": [ { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "A state representing a view", "genericTokens": undefined, "name": "NavigationFlowViewState", @@ -1978,7 +1980,9 @@ If the expression is a composite, the last expression executed is the return val "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "An END state of the flow.", "genericTokens": undefined, "name": "NavigationFlowEndState", @@ -2347,7 +2351,9 @@ The return value determines the transition to take", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "External Flow states represent states in the FSM that can't be resolved internally in Player. The flow will wait for the embedded application to manage moving to the next state via a transition", "genericTokens": undefined, @@ -2643,7 +2649,9 @@ The flow will wait for the embedded application to manage moving to the next sta "name": "NavigationFlowState", "or": [ { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "A state representing a view", "name": "NavigationFlowViewState", "properties": { @@ -2781,7 +2789,9 @@ The flow will wait for the embedded application to manage moving to the next sta "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "An END state of the flow.", "name": "NavigationFlowEndState", "properties": { @@ -3138,7 +3148,9 @@ The return value determines the transition to take", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "External Flow states represent states in the FSM that can't be resolved internally in Player. The flow will wait for the embedded application to manage moving to the next state via a transition", "name": "NavigationFlowExternalState", @@ -3657,7 +3669,9 @@ So this explicity says there should never be an exp prop on a state node that"s "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "A state representing a view", "genericTokens": undefined, "name": "NavigationFlowViewState", @@ -3799,7 +3813,9 @@ So this explicity says there should never be an exp prop on a state node that"s "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "An END state of the flow.", "genericTokens": undefined, "name": "NavigationFlowEndState", @@ -4038,7 +4054,9 @@ The return value determines the transition to take", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "External Flow states represent states in the FSM that can't be resolved internally in Player. The flow will wait for the embedded application to manage moving to the next state via a transition", "genericTokens": undefined, @@ -4302,7 +4320,9 @@ The flow will wait for the embedded application to manage moving to the next sta "name": "NavigationFlowState", "or": [ { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "A state representing a view", "name": "NavigationFlowViewState", "properties": { @@ -4440,7 +4460,9 @@ The flow will wait for the embedded application to manage moving to the next sta "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "An END state of the flow.", "name": "NavigationFlowEndState", "properties": { @@ -4797,7 +4819,9 @@ The return value determines the transition to take", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "External Flow states represent states in the FSM that can't be resolved internally in Player. The flow will wait for the embedded application to manage moving to the next state via a transition", "name": "NavigationFlowExternalState", @@ -4944,7 +4968,9 @@ The flow will wait for the embedded application to manage moving to the next sta }, "endState": { "node": { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "The outcome describes _how_ the flow ended (forwards, backwards, etc)", "genericTokens": undefined, "name": "NavigationFlowEndState", @@ -5507,7 +5533,9 @@ This will be used to lookup the proper handler", "name": "NavigationFlowState", "or": [ { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "A state representing a view", "name": "NavigationFlowViewState", "properties": { @@ -5645,7 +5673,9 @@ This will be used to lookup the proper handler", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "An END state of the flow.", "name": "NavigationFlowEndState", "properties": { @@ -6002,7 +6032,9 @@ The return value determines the transition to take", "type": "object", }, { - "additionalProperties": false, + "additionalProperties": { + "type": "unknown", + }, "description": "External Flow states represent states in the FSM that can't be resolved internally in Player. The flow will wait for the embedded application to manage moving to the next state via a transition", "name": "NavigationFlowExternalState", diff --git a/xlr/converters/src/__tests__/heritage-additional-properties.test.ts b/xlr/converters/src/__tests__/heritage-additional-properties.test.ts new file mode 100644 index 00000000..d2d52c92 --- /dev/null +++ b/xlr/converters/src/__tests__/heritage-additional-properties.test.ts @@ -0,0 +1,119 @@ +import { test, expect } from "vitest"; +import { setupTestEnv } from "@player-tools/test-utils"; +import { TsConverter } from ".."; + +/** + * Tests for handleHeritageClauses: when an interface extends a base that has + * no index signature, but the current interface declares [key: string]: unknown, + * the merged XLR must include the current interface's additionalProperties + * (so the merged type allows extra keys). + */ +test("merged type includes current interface additionalProperties when extending base without index signature", () => { + const sc = ` + interface NavigationBaseState { + state_type: string; + onStart?: string; + } + + export interface NavigationFlowEndState extends NavigationBaseState { + outcome: string; + [key: string]: unknown; + } + `; + + const { sf, tc } = setupTestEnv(sc); + const converter = new TsConverter(tc); + const types = converter.convertSourceFile(sf).data.types; + + const endStateType = types.find( + (t: { name?: string }) => t.name === "NavigationFlowEndState", + ); + expect(endStateType).toBeDefined(); + expect( + (endStateType as { additionalProperties?: unknown }).additionalProperties, + ).not.toBe(false); + expect( + ( + (endStateType as { additionalProperties?: { type?: string } }) + .additionalProperties as { type?: string } + )?.type, + ).toBe("unknown"); +}); + +test("merged type includes additionalProperties when only extended type has index signature", () => { + const sc = ` + interface Base { + required: string; + } + + interface WithIndex { + [key: string]: unknown; + } + + export interface Child extends Base, WithIndex { + extra: number; + } + `; + + const { sf, tc } = setupTestEnv(sc); + const converter = new TsConverter(tc); + const types = converter.convertSourceFile(sf).data.types; + + const childType = types.find((t: { name?: string }) => t.name === "Child"); + expect(childType).toBeDefined(); + const additionalProperties = (childType as { additionalProperties?: unknown }) + .additionalProperties; + expect(additionalProperties).not.toBe(false); + expect((additionalProperties as { type?: string })?.type).toBe("unknown"); +}); + +test("merged type has no additionalProperties when no interface has index signature", () => { + const sc = ` + interface Base { + required: string; + } + + export interface Child extends Base { + extra: number; + } + `; + + const { sf, tc } = setupTestEnv(sc); + const converter = new TsConverter(tc); + const types = converter.convertSourceFile(sf).data.types; + + const childType = types.find((t: { name?: string }) => t.name === "Child"); + expect(childType).toBeDefined(); + const additionalProperties = (childType as { additionalProperties?: unknown }) + .additionalProperties; + expect(additionalProperties).toBe(false); +}); + +test("merged type combines additionalProperties from current and extended when both have index signatures", () => { + const sc = ` + interface Base { + required: string; + [key: string]: unknown; + } + + export interface Child extends Base { + extra: number; + [key: string]: unknown; + } + `; + + const { sf, tc } = setupTestEnv(sc); + const converter = new TsConverter(tc); + const types = converter.convertSourceFile(sf).data.types; + + const childType = types.find((t: { name?: string }) => t.name === "Child"); + expect(childType).toBeDefined(); + const additionalProperties = (childType as { additionalProperties?: unknown }) + .additionalProperties; + expect(additionalProperties).not.toBe(false); + // When both have index sigs we merge (e.g. or of both); extra keys are allowed + const ap = additionalProperties as { type?: string; or?: unknown[] }; + expect( + ap.type === "unknown" || (ap.type === "or" && Array.isArray(ap.or)), + ).toBe(true); +}); diff --git a/xlr/converters/src/ts-to-xlr.ts b/xlr/converters/src/ts-to-xlr.ts index 47cfb9ea..7926d531 100644 --- a/xlr/converters/src/ts-to-xlr.ts +++ b/xlr/converters/src/ts-to-xlr.ts @@ -915,17 +915,29 @@ export class TsConverter { } }); }); - // Resolve Additional Properties + // Resolve Additional Properties: preserve index signature from current + // interface (baseObject) and merge with any from extended types. let additionalProperties: NodeType | false = false; - if (baseObject.additionalProperties === false) { - if (additionalPropertiesCollector.length === 1) { - additionalProperties = additionalPropertiesCollector[0]; - } else if (additionalPropertiesCollector.length >= 1) { - additionalProperties = { - type: "or", - or: additionalPropertiesCollector, - }; + if (baseObject.additionalProperties) { + if (additionalPropertiesCollector.length === 0) { + additionalProperties = baseObject.additionalProperties; + } else { + additionalPropertiesCollector.push(baseObject.additionalProperties); + additionalProperties = + additionalPropertiesCollector.length === 1 + ? additionalPropertiesCollector[0] + : { + type: "or", + or: additionalPropertiesCollector, + }; } + } else if (additionalPropertiesCollector.length === 1) { + additionalProperties = additionalPropertiesCollector[0]; + } else if (additionalPropertiesCollector.length >= 1) { + additionalProperties = { + type: "or", + or: additionalPropertiesCollector, + }; } return { diff --git a/xlr/utils/src/__tests__/ts-helpers.test.ts b/xlr/utils/src/__tests__/ts-helpers.test.ts index 95b3cdd4..cced2ae6 100644 --- a/xlr/utils/src/__tests__/ts-helpers.test.ts +++ b/xlr/utils/src/__tests__/ts-helpers.test.ts @@ -1,6 +1,6 @@ import { test, expect, describe } from "vitest"; import * as ts from "typescript"; -import { NodeType } from "@player-tools/xlr"; +import type { NodeType } from "@player-tools/xlr"; import { tsStripOptionalType,