From c028ca9f95e9a4704c7fa8d06ba6bef860ad16da Mon Sep 17 00:00:00 2001 From: justin-medeiros Date: Fri, 20 Feb 2026 15:34:03 -0500 Subject: [PATCH 1/4] Fix validator throwing incorrect errors --- .../heritage-additional-properties.test.ts | 119 ++++++++++++++++++ xlr/converters/src/ts-to-xlr.ts | 30 +++-- xlr/utils/src/__tests__/ts-helpers.test.ts | 81 +++++++++++- xlr/utils/src/ts-helpers.ts | 25 +++- 4 files changed, 240 insertions(+), 15 deletions(-) create mode 100644 xlr/converters/src/__tests__/heritage-additional-properties.test.ts 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..1c6a4c0f 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 { NodeType, ObjectType } from "@player-tools/xlr"; import { tsStripOptionalType, @@ -8,6 +8,7 @@ import { applyPickOrOmitToNodeType, getStringLiteralsFromUnion, applyPartialOrRequiredToNodeType, + fillInGenerics, } from "../ts-helpers"; test("tsStripOptionalType", () => { @@ -178,3 +179,81 @@ describe("applyPartialOrRequiredToNodeType", () => { expect(result).toStrictEqual(modifiedObject); }); }); + +describe("fillInGenerics", () => { + test("scopes refs inside object with genericTokens to inner generic, not top-level", () => { + // Simulates Schema.DataType nested under Flow: + // inner object has T = unknown; we pass top-level map T → Asset. + // Refs inside the inner object (e.g. default: ref "T") must resolve to unknown. + const assetLikeType: NodeType = { + type: "object", + properties: { + id: { required: true, node: { type: "string" } }, + type: { required: true, node: { type: "string" } }, + }, + additionalProperties: false, + }; + const topLevelGenerics = new Map([["T", assetLikeType]]); + + const dataTypeLike: NodeType = { + type: "object", + name: "DataType", + properties: { + type: { + required: true, + node: { type: "string" }, + }, + default: { + required: false, + node: { type: "ref", ref: "T" }, + }, + }, + additionalProperties: false, + genericTokens: [ + { symbol: "T", constraints: undefined, default: { type: "unknown" } }, + ], + } as NodeType; + + const result = fillInGenerics(dataTypeLike, topLevelGenerics); + + expect(result.type).toBe("object"); + const resultObj = result as ObjectType; + expect(resultObj.properties.default.node).toBeDefined(); + // Resolved from inner scope (DataType's T = unknown), not top-level (Asset) + expect((resultObj.properties.default.node as { type?: string }).type).toBe( + "unknown", + ); + }); + + test("uses passed-in generic map when object has no genericTokens", () => { + const assetLikeType: NodeType = { + type: "object", + properties: { + id: { required: true, node: { type: "string" } }, + }, + additionalProperties: false, + }; + const generics = new Map([["T", assetLikeType]]); + + // No genericTokens on this object (e.g. Flow level): passed-in map is used, + // so ref "T" resolves to Asset — Flow still works as T = Asset. + const nodeWithRefT: NodeType = { + type: "object", + properties: { + value: { + required: true, + node: { type: "ref", ref: "T" }, + }, + }, + additionalProperties: false, + }; + + const result = fillInGenerics(nodeWithRefT, generics); + + const resultObj = result as ObjectType; + expect(resultObj.properties.value.node).toBeDefined(); + expect((resultObj.properties.value.node as { type?: string }).type).toBe( + "object", + ); + }); +}); diff --git a/xlr/utils/src/ts-helpers.ts b/xlr/utils/src/ts-helpers.ts index 793032a2..e29ca8d0 100644 --- a/xlr/utils/src/ts-helpers.ts +++ b/xlr/utils/src/ts-helpers.ts @@ -228,12 +228,27 @@ export function fillInGenerics( } if (xlrNode.type === "object") { + // When an object has its own genericTokens, scope refs inside it to those + // tokens (e.g. DataType's T) so they don't incorrectly resolve to an outer + // type's generics (e.g. Flow's T = Asset). + let scopeGenerics = localGenerics; + if (isGenericNamedType(xlrNode) && xlrNode.genericTokens) { + scopeGenerics = new Map(localGenerics); + xlrNode.genericTokens.forEach((token) => { + const genericValue = (token.default ?? token.constraints) as NodeType; + scopeGenerics.set( + token.symbol, + fillInGenerics(genericValue, scopeGenerics), + ); + }); + } + const newProperties: { [name: string]: ObjectProperty } = {}; Object.getOwnPropertyNames(xlrNode.properties).forEach((propName) => { const prop = xlrNode.properties[propName]; newProperties[propName] = { required: prop.required, - node: fillInGenerics(prop.node, localGenerics), + node: fillInGenerics(prop.node, scopeGenerics), }; }); @@ -246,20 +261,20 @@ export function fillInGenerics( return { ...token, constraints: token.constraints - ? fillInGenerics(token.constraints, localGenerics) + ? fillInGenerics(token.constraints, scopeGenerics) : undefined, default: token.default - ? fillInGenerics(token.default, localGenerics) + ? fillInGenerics(token.default, scopeGenerics) : undefined, }; }), } : {}), extends: xlrNode.extends - ? (fillInGenerics(xlrNode.extends, localGenerics) as RefNode) + ? (fillInGenerics(xlrNode.extends, scopeGenerics) as RefNode) : undefined, additionalProperties: xlrNode.additionalProperties - ? fillInGenerics(xlrNode.additionalProperties, localGenerics) + ? fillInGenerics(xlrNode.additionalProperties, scopeGenerics) : false, }; } From 36aaff5f1e6f127f8e49adee51893f1b2d8ed4ec Mon Sep 17 00:00:00 2001 From: justin-medeiros Date: Fri, 20 Feb 2026 16:23:56 -0500 Subject: [PATCH 2/4] Test --- .../__snapshots__/player.test.ts.snap | 64 ++++++-- xlr/utils/src/__tests__/ts-helpers.test.ts | 154 +++++++++--------- xlr/utils/src/ts-helpers.ts | 32 ++-- 3 files changed, 141 insertions(+), 109 deletions(-) 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/utils/src/__tests__/ts-helpers.test.ts b/xlr/utils/src/__tests__/ts-helpers.test.ts index 1c6a4c0f..59c55192 100644 --- a/xlr/utils/src/__tests__/ts-helpers.test.ts +++ b/xlr/utils/src/__tests__/ts-helpers.test.ts @@ -180,80 +180,80 @@ describe("applyPartialOrRequiredToNodeType", () => { }); }); -describe("fillInGenerics", () => { - test("scopes refs inside object with genericTokens to inner generic, not top-level", () => { - // Simulates Schema.DataType nested under Flow: - // inner object has T = unknown; we pass top-level map T → Asset. - // Refs inside the inner object (e.g. default: ref "T") must resolve to unknown. - const assetLikeType: NodeType = { - type: "object", - properties: { - id: { required: true, node: { type: "string" } }, - type: { required: true, node: { type: "string" } }, - }, - additionalProperties: false, - }; - const topLevelGenerics = new Map([["T", assetLikeType]]); - - const dataTypeLike: NodeType = { - type: "object", - name: "DataType", - properties: { - type: { - required: true, - node: { type: "string" }, - }, - default: { - required: false, - node: { type: "ref", ref: "T" }, - }, - }, - additionalProperties: false, - genericTokens: [ - { symbol: "T", constraints: undefined, default: { type: "unknown" } }, - ], - } as NodeType; - - const result = fillInGenerics(dataTypeLike, topLevelGenerics); - - expect(result.type).toBe("object"); - const resultObj = result as ObjectType; - expect(resultObj.properties.default.node).toBeDefined(); - // Resolved from inner scope (DataType's T = unknown), not top-level (Asset) - expect((resultObj.properties.default.node as { type?: string }).type).toBe( - "unknown", - ); - }); - - test("uses passed-in generic map when object has no genericTokens", () => { - const assetLikeType: NodeType = { - type: "object", - properties: { - id: { required: true, node: { type: "string" } }, - }, - additionalProperties: false, - }; - const generics = new Map([["T", assetLikeType]]); - - // No genericTokens on this object (e.g. Flow level): passed-in map is used, - // so ref "T" resolves to Asset — Flow still works as T = Asset. - const nodeWithRefT: NodeType = { - type: "object", - properties: { - value: { - required: true, - node: { type: "ref", ref: "T" }, - }, - }, - additionalProperties: false, - }; - - const result = fillInGenerics(nodeWithRefT, generics); - - const resultObj = result as ObjectType; - expect(resultObj.properties.value.node).toBeDefined(); - expect((resultObj.properties.value.node as { type?: string }).type).toBe( - "object", - ); - }); -}); +// describe("fillInGenerics", () => { +// test("scopes refs inside object with genericTokens to inner generic, not top-level", () => { +// // Simulates Schema.DataType nested under Flow: +// // inner object has T = unknown; we pass top-level map T → Asset. +// // Refs inside the inner object (e.g. default: ref "T") must resolve to unknown. +// const assetLikeType: NodeType = { +// type: "object", +// properties: { +// id: { required: true, node: { type: "string" } }, +// type: { required: true, node: { type: "string" } }, +// }, +// additionalProperties: false, +// }; +// const topLevelGenerics = new Map([["T", assetLikeType]]); + +// const dataTypeLike: NodeType = { +// type: "object", +// name: "DataType", +// properties: { +// type: { +// required: true, +// node: { type: "string" }, +// }, +// default: { +// required: false, +// node: { type: "ref", ref: "T" }, +// }, +// }, +// additionalProperties: false, +// genericTokens: [ +// { symbol: "T", constraints: undefined, default: { type: "unknown" } }, +// ], +// } as NodeType; + +// const result = fillInGenerics(dataTypeLike, topLevelGenerics); + +// expect(result.type).toBe("object"); +// const resultObj = result as ObjectType; +// expect(resultObj.properties.default.node).toBeDefined(); +// // Resolved from inner scope (DataType's T = unknown), not top-level (Asset) +// expect((resultObj.properties.default.node as { type?: string }).type).toBe( +// "unknown", +// ); +// }); + +// test("uses passed-in generic map when object has no genericTokens", () => { +// const assetLikeType: NodeType = { +// type: "object", +// properties: { +// id: { required: true, node: { type: "string" } }, +// }, +// additionalProperties: false, +// }; +// const generics = new Map([["T", assetLikeType]]); + +// // No genericTokens on this object (e.g. Flow level): passed-in map is used, +// // so ref "T" resolves to Asset — Flow still works as T = Asset. +// const nodeWithRefT: NodeType = { +// type: "object", +// properties: { +// value: { +// required: true, +// node: { type: "ref", ref: "T" }, +// }, +// }, +// additionalProperties: false, +// }; + +// const result = fillInGenerics(nodeWithRefT, generics); + +// const resultObj = result as ObjectType; +// expect(resultObj.properties.value.node).toBeDefined(); +// expect((resultObj.properties.value.node as { type?: string }).type).toBe( +// "object", +// ); +// }); +// }); diff --git a/xlr/utils/src/ts-helpers.ts b/xlr/utils/src/ts-helpers.ts index e29ca8d0..03c8f0dc 100644 --- a/xlr/utils/src/ts-helpers.ts +++ b/xlr/utils/src/ts-helpers.ts @@ -231,24 +231,24 @@ export function fillInGenerics( // When an object has its own genericTokens, scope refs inside it to those // tokens (e.g. DataType's T) so they don't incorrectly resolve to an outer // type's generics (e.g. Flow's T = Asset). - let scopeGenerics = localGenerics; - if (isGenericNamedType(xlrNode) && xlrNode.genericTokens) { - scopeGenerics = new Map(localGenerics); - xlrNode.genericTokens.forEach((token) => { - const genericValue = (token.default ?? token.constraints) as NodeType; - scopeGenerics.set( - token.symbol, - fillInGenerics(genericValue, scopeGenerics), - ); - }); - } + // let scopeGenerics = localGenerics; + // if (isGenericNamedType(xlrNode) && xlrNode.genericTokens) { + // scopeGenerics = new Map(localGenerics); + // xlrNode.genericTokens.forEach((token) => { + // const genericValue = (token.default ?? token.constraints) as NodeType; + // scopeGenerics.set( + // token.symbol, + // fillInGenerics(genericValue, scopeGenerics), + // ); + // }); + // } const newProperties: { [name: string]: ObjectProperty } = {}; Object.getOwnPropertyNames(xlrNode.properties).forEach((propName) => { const prop = xlrNode.properties[propName]; newProperties[propName] = { required: prop.required, - node: fillInGenerics(prop.node, scopeGenerics), + node: fillInGenerics(prop.node, localGenerics), }; }); @@ -261,20 +261,20 @@ export function fillInGenerics( return { ...token, constraints: token.constraints - ? fillInGenerics(token.constraints, scopeGenerics) + ? fillInGenerics(token.constraints, localGenerics) : undefined, default: token.default - ? fillInGenerics(token.default, scopeGenerics) + ? fillInGenerics(token.default, localGenerics) : undefined, }; }), } : {}), extends: xlrNode.extends - ? (fillInGenerics(xlrNode.extends, scopeGenerics) as RefNode) + ? (fillInGenerics(xlrNode.extends, localGenerics) as RefNode) : undefined, additionalProperties: xlrNode.additionalProperties - ? fillInGenerics(xlrNode.additionalProperties, scopeGenerics) + ? fillInGenerics(xlrNode.additionalProperties, localGenerics) : false, }; } From 44c47e7b5290cc160230a8625ef7f6ac4000e7f4 Mon Sep 17 00:00:00 2001 From: justin-medeiros Date: Fri, 20 Feb 2026 17:19:54 -0500 Subject: [PATCH 3/4] Revert scopeGenerics --- xlr/utils/src/__tests__/ts-helpers.test.ts | 3 +-- xlr/utils/src/ts-helpers.ts | 15 --------------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/xlr/utils/src/__tests__/ts-helpers.test.ts b/xlr/utils/src/__tests__/ts-helpers.test.ts index 59c55192..6008dc35 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, ObjectType } from "@player-tools/xlr"; +import type { NodeType } from "@player-tools/xlr"; import { tsStripOptionalType, @@ -8,7 +8,6 @@ import { applyPickOrOmitToNodeType, getStringLiteralsFromUnion, applyPartialOrRequiredToNodeType, - fillInGenerics, } from "../ts-helpers"; test("tsStripOptionalType", () => { diff --git a/xlr/utils/src/ts-helpers.ts b/xlr/utils/src/ts-helpers.ts index 03c8f0dc..793032a2 100644 --- a/xlr/utils/src/ts-helpers.ts +++ b/xlr/utils/src/ts-helpers.ts @@ -228,21 +228,6 @@ export function fillInGenerics( } if (xlrNode.type === "object") { - // When an object has its own genericTokens, scope refs inside it to those - // tokens (e.g. DataType's T) so they don't incorrectly resolve to an outer - // type's generics (e.g. Flow's T = Asset). - // let scopeGenerics = localGenerics; - // if (isGenericNamedType(xlrNode) && xlrNode.genericTokens) { - // scopeGenerics = new Map(localGenerics); - // xlrNode.genericTokens.forEach((token) => { - // const genericValue = (token.default ?? token.constraints) as NodeType; - // scopeGenerics.set( - // token.symbol, - // fillInGenerics(genericValue, scopeGenerics), - // ); - // }); - // } - const newProperties: { [name: string]: ObjectProperty } = {}; Object.getOwnPropertyNames(xlrNode.properties).forEach((propName) => { const prop = xlrNode.properties[propName]; From 1b66fd3f634528bfc79fc3ed1477bc5dbb40d27e Mon Sep 17 00:00:00 2001 From: justin-medeiros Date: Fri, 20 Feb 2026 17:22:09 -0500 Subject: [PATCH 4/4] Remove commented out test --- xlr/utils/src/__tests__/ts-helpers.test.ts | 78 ---------------------- 1 file changed, 78 deletions(-) diff --git a/xlr/utils/src/__tests__/ts-helpers.test.ts b/xlr/utils/src/__tests__/ts-helpers.test.ts index 6008dc35..cced2ae6 100644 --- a/xlr/utils/src/__tests__/ts-helpers.test.ts +++ b/xlr/utils/src/__tests__/ts-helpers.test.ts @@ -178,81 +178,3 @@ describe("applyPartialOrRequiredToNodeType", () => { expect(result).toStrictEqual(modifiedObject); }); }); - -// describe("fillInGenerics", () => { -// test("scopes refs inside object with genericTokens to inner generic, not top-level", () => { -// // Simulates Schema.DataType nested under Flow: -// // inner object has T = unknown; we pass top-level map T → Asset. -// // Refs inside the inner object (e.g. default: ref "T") must resolve to unknown. -// const assetLikeType: NodeType = { -// type: "object", -// properties: { -// id: { required: true, node: { type: "string" } }, -// type: { required: true, node: { type: "string" } }, -// }, -// additionalProperties: false, -// }; -// const topLevelGenerics = new Map([["T", assetLikeType]]); - -// const dataTypeLike: NodeType = { -// type: "object", -// name: "DataType", -// properties: { -// type: { -// required: true, -// node: { type: "string" }, -// }, -// default: { -// required: false, -// node: { type: "ref", ref: "T" }, -// }, -// }, -// additionalProperties: false, -// genericTokens: [ -// { symbol: "T", constraints: undefined, default: { type: "unknown" } }, -// ], -// } as NodeType; - -// const result = fillInGenerics(dataTypeLike, topLevelGenerics); - -// expect(result.type).toBe("object"); -// const resultObj = result as ObjectType; -// expect(resultObj.properties.default.node).toBeDefined(); -// // Resolved from inner scope (DataType's T = unknown), not top-level (Asset) -// expect((resultObj.properties.default.node as { type?: string }).type).toBe( -// "unknown", -// ); -// }); - -// test("uses passed-in generic map when object has no genericTokens", () => { -// const assetLikeType: NodeType = { -// type: "object", -// properties: { -// id: { required: true, node: { type: "string" } }, -// }, -// additionalProperties: false, -// }; -// const generics = new Map([["T", assetLikeType]]); - -// // No genericTokens on this object (e.g. Flow level): passed-in map is used, -// // so ref "T" resolves to Asset — Flow still works as T = Asset. -// const nodeWithRefT: NodeType = { -// type: "object", -// properties: { -// value: { -// required: true, -// node: { type: "ref", ref: "T" }, -// }, -// }, -// additionalProperties: false, -// }; - -// const result = fillInGenerics(nodeWithRefT, generics); - -// const resultObj = result as ObjectType; -// expect(resultObj.properties.value.node).toBeDefined(); -// expect((resultObj.properties.value.node as { type?: string }).type).toBe( -// "object", -// ); -// }); -// });