Skip to content

Fix additionalProperties in XLR types #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
justin-medeiros wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix additionalProperties in XLR types #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c028ca9
Fix validator throwing incorrect errors
justin-medeiros Feb 20, 2026
36aaff5
Test
justin-medeiros Feb 20, 2026
44c47e7
Revert scopeGenerics
justin-medeiros Feb 20, 2026
1b66fd3
Remove commented out test
justin-medeiros Feb 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 48 additions & 16 deletions xlr/converters/src/__tests__/__snapshots__/player.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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",
Expand Down
119 changes: 119 additions & 0 deletions xlr/converters/src/__tests__/heritage-additional-properties.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
30 changes: 21 additions & 9 deletions xlr/converters/src/ts-to-xlr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion xlr/utils/src/__tests__/ts-helpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand Down