diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts index d5d55bba..a2127f2b 100644 --- a/clients/web/src/test/core/nullableUnion.test.ts +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -390,6 +390,70 @@ describe("normalizeNullableUnion", () => { expect(normalizeNullableUnion(schema)).toBe(schema); }); + // The hoist is a spread, but JSON Schema applies sibling keywords + // conjunctively — so a branch keyword replacing the wrapper's would *widen* + // the field. `enum: ["a"]` around a branch `enum: ["a","b"]` means "a", yet + // the spread would offer "b" in the dropdown and submit it. + it("declines when the branch would drop a conflicting wrapper enum", () => { + const schema = { + enum: ["a"], + anyOf: [{ type: "string", enum: ["a", "b"] }, { type: "null" as const }], + }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("declines when wrapper and branch disagree on type", () => { + const schema = { + type: "string", + anyOf: [{ type: "integer" }, { type: "null" as const }], + }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + it("declines when wrapper and branch disagree on bounds", () => { + const schema = { + minimum: 5, + anyOf: [{ type: "integer", minimum: 1 }, { type: "null" as const }], + }; + expect(normalizeNullableUnion(schema)).toBe(schema); + }); + + // Identical values are no conflict, so the ordinary redundant-type shape + // still collapses rather than being punished for being explicit. + it("collapses when wrapper and branch agree on a shared keyword", () => { + expect( + normalizeNullableUnion({ + type: "string", + anyOf: [{ type: "string", enum: ["a"] }, { type: "null" }], + }), + ).toEqual({ + type: "string", + enum: ["a"], + anyOf: undefined, + // The explicit non-null `type` rules null out, conjunctively. + nullable: false, + }); + }); + + it("treats wrapper metadata as safe to keep alongside branch constraints", () => { + expect( + normalizeNullableUnion({ + title: "Direction", + description: "Which way", + default: "envio", + anyOf: [{ type: "string", enum: ["envio"] }, { type: "null" }], + }), + ).toEqual({ + title: "Direction", + description: "Which way", + default: "envio", + type: "string", + enum: ["envio"], + anyOf: undefined, + nullable: true, + }); + }); + it("ignores a schema with no union keywords at all", () => { const schema = { type: "object" as const, properties: {} }; expect(normalizeNullableUnion(schema)).toBe(schema); @@ -470,6 +534,30 @@ describe("admitsNull", () => { // A branch's enum is scoped to that branch, not a sibling of the union — this // is the #1928 shape, and reading it as a sibling would call it non-nullable. + // An explicit `type` is conjunctive with the union, so it decides rather than + // merely offering another way to say yes. A `{ type: "null" }` branch cannot + // override a top-level `type: "string"`. + it("lets an explicit non-null type override a null branch", () => { + expect( + admitsNull({ + type: "string", + anyOf: [{ type: "string", enum: ["a"] }, { type: "null" }], + }), + ).toBe(false); + }); + + it("still honors an explicit type that does admit null", () => { + expect( + admitsNull({ + type: ["string", "null"], + anyOf: [{ type: "string" }, { type: "null" }], + }), + ).toBe(true); + expect(admitsNull({ type: "null", anyOf: [{ type: "string" }] })).toBe( + true, + ); + }); + it("is unaffected by an enum that lives inside an anyOf branch", () => { expect( admitsNull({ diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts index 6141bcdd..c24fec92 100644 --- a/core/json/nullableUnion.ts +++ b/core/json/nullableUnion.ts @@ -126,6 +126,72 @@ function nullExcludedBySiblings(schema: NullableUnionSchema): boolean { return schema.const !== undefined && schema.const !== null; } +/** + * Keywords that *constrain what values are valid*, as opposed to describing the + * field (`title`, `description`, `default`, `enumNames`). + * + * Only these can conflict in a way that matters: a wrapper and its branch each + * carrying one means the value must satisfy **both**, which the hoist cannot + * express. Metadata is safe to merge because nothing validates against it. + */ +const VALIDATION_KEYWORDS = [ + "type", + "enum", + "const", + "items", + "properties", + "required", + "additionalProperties", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "pattern", + "format", + "minItems", + "maxItems", + "uniqueItems", +] as const; + +/** + * Whether hoisting this branch would *drop* one of the wrapper's constraints. + * + * The hoist is a spread, so a branch key replaces the wrapper's — but JSON + * Schema applies sibling keywords **conjunctively**, and a replacement is not a + * conjunction. A wrapper `enum: ["a"]` around a branch `enum: ["a", "b"]` means + * `"a"` (both must hold), yet the spread yields `["a", "b"]` and the rendered + * dropdown would offer — and submit — a `"b"` the schema rejects. `type`, + * bounds, and the object keywords fail the same way. + * + * Rather than implement intersection for every keyword (a much larger surface, + * and one where a subtly wrong intersection is worse than none), the collapse + * **declines** when wrapper and branch disagree, and the field renders through + * the JSON editor with its full schema intact. Identical values are not a + * conflict, so the common `{ type: "string", anyOf: [{ type: "string", … }] }` + * still collapses. + * + * Comparison is by canonical JSON, so two structurally equal objects written + * with different key order read as a conflict. That is conservative in the safe + * direction: the field falls back rather than mis-renders. + */ +function branchDropsWrapperConstraint( + schema: NullableUnionSchema, + branch: Record, +): boolean { + const wrapper = schema as Record; + return VALIDATION_KEYWORDS.some((keyword) => { + const wrapperValue = wrapper[keyword]; + const branchValue = branch[keyword]; + if (wrapperValue === undefined || branchValue === undefined) { + return false; + } + return JSON.stringify(wrapperValue) !== JSON.stringify(branchValue); + }); +} + /** * What {@link stripNullEnumMembers} concluded about an `enum`. * @@ -250,11 +316,15 @@ export function admitsNull(schema: NullableUnionSchema): boolean { if (schema.nullable === true) { return true; } - if (schema.type === "null") { - return true; - } - if (Array.isArray(schema.type) && schema.type.includes("null")) { - return true; + // An explicit `type` is a sibling constraint too, so it *decides* — it does + // not merely add a way to say yes. `{ type: "string", anyOf: [..., { type: + // "null" }] }` rejects null, because a value must satisfy the `type` as well + // as the union. Falling through to the branch scan here would let the union + // override a constraint that outranks it. + if (schema.type !== undefined) { + return Array.isArray(schema.type) + ? schema.type.includes("null") + : schema.type === "null"; } return [schema.anyOf, schema.oneOf].some((branches) => branches?.some((entry) => { @@ -303,7 +373,9 @@ export function normalizeNullableUnion( (entry) => entry !== null && entry.type !== "null", ); - if (nullBranch && branch) { + // Hoisting would silently drop a wrapper constraint the branch also + // carries, so decline instead; see `branchDropsWrapperConstraint`. + if (nullBranch && branch && !branchDropsWrapperConstraint(schema, branch)) { // A branch may carry an `enum` and no `type`; JSON Schema allows that, and // an all-string enum is unambiguously a string field. See isStringEnum for // why a non-string enum deliberately does not get the same treatment.