From b710af2d292a8a9ff129424016b1a0d426ab4877 Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Wed, 23 Sep 2026 14:10:32 -0300 Subject: [PATCH] fix: keep a `false` property subschema when a conditional branch adds a constraint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two allOf branches touch the same property — one setting it to `false` (forbidden) and another adding a constraint like `{ maximum }` — mergeSchemaBranch overwrote the `false` with the constraint object. The property then rendered as a normal visible field and validation accepted a value the schema forbids. A `false` property subschema is unsatisfiable, so allOf semantics keep the property forbidden regardless of sibling constraints. Keep the `false` when merging property subschemas. The check is scoped to property maps so a branch can still relax a boolean keyword such as `additionalProperties: false`. Fixes both the field-build path (field is now hidden) and the validation path (a value for the forbidden property is now rejected), covered by unit tests on mergeSchemaBranch and an integration test through createHeadlessForm. --- src/utils.ts | 14 ++++++++-- test/fields/visibility.test.ts | 47 ++++++++++++++++++++++++++++++++++ test/utils.test.ts | 29 +++++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index a5869870..6bb03f36 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -113,7 +113,7 @@ function warnAboutNewConditionalOptions(newOptions: unknown[]): void { * @param schema2 - The conditional branch schema to merge from * @param options - The form options */ -export function mergeSchemaBranch>(schema1?: T, schema2?: T, options?: CreateHeadlessFormOptions): void { +export function mergeSchemaBranch>(schema1?: T, schema2?: T, options?: CreateHeadlessFormOptions, insidePropertyMap: boolean = false): void { // Handle null/undefined values if (!schema1 || !schema2) { return @@ -135,6 +135,14 @@ export function mergeSchemaBranch>(schema1?: T, sc const schema1Value = schema1[key] + // A `false` property subschema is unsatisfiable, so `allOf` semantics keep the property + // forbidden no matter what a sibling branch adds; otherwise a branch object overwrites the + // `false` and the property wrongly becomes valid again. Scoped to property subschemas so a + // branch can still relax a boolean keyword like `additionalProperties: false`. + if (insidePropertyMap && schema1Value === false) { + continue + } + if (isOptionsLikeSchema(key, schema2Value)) { // Restrict option-like arrays to the options already present on the base field if (disallowNewConditionalOptions) { @@ -171,7 +179,9 @@ export function mergeSchemaBranch>(schema1?: T, sc if (isObject(schema2Value)) { // If both schemas have this key and it's an object, merge recursively if (isObject(schema1Value)) { - mergeSchemaBranch(schema1Value, schema2Value, options) + // The direct children of `properties`/`patternProperties` are property subschemas, where a + // `false` value forbids the property and must survive a sibling branch. + mergeSchemaBranch(schema1Value, schema2Value, options, key === 'properties' || key === 'patternProperties') } // Otherwise, if the value is different, just assign it else if (schema1Value !== schema2Value) { diff --git a/test/fields/visibility.test.ts b/test/fields/visibility.test.ts index b3915d1a..5f213369 100644 --- a/test/fields/visibility.test.ts +++ b/test/fields/visibility.test.ts @@ -744,4 +744,51 @@ describe('Field visibility', () => { }) }) }) + + describe('a forbidden property with a sibling branch constraining the same field', () => { + const schema: JsfObjectSchema = { + type: 'object', + properties: { + contract_duration_type: { + type: 'string', + oneOf: [{ const: 'indefinite' }, { const: 'fixed_term' }], + }, + months: { + type: 'number', + minimum: 1, + }, + }, + required: ['contract_duration_type'], + allOf: [ + { + if: { + properties: { contract_duration_type: { const: 'fixed_term' } }, + required: ['contract_duration_type'], + }, + then: { required: ['months'] }, + else: { properties: { months: false } }, + }, + { + if: true, + then: { properties: { months: { maximum: 12 } } }, + }, + ], + } + + it('hides the forbidden field and rejects a value for it', () => { + const form = createHeadlessForm(schema, { initialValues: { contract_duration_type: 'indefinite' } }) + expect(getField(form.fields, 'months')?.isVisible).toBe(false) + + const { formErrors } = form.handleValidation({ contract_duration_type: 'indefinite', months: 5 }) + expect(formErrors?.months).toBeDefined() + }) + + it('shows the field and accepts a value when the branch requires it', () => { + const form = createHeadlessForm(schema, { initialValues: { contract_duration_type: 'fixed_term' } }) + expect(getField(form.fields, 'months')?.isVisible).toBe(true) + + const { formErrors } = form.handleValidation({ contract_duration_type: 'fixed_term', months: 6 }) + expect(formErrors).toBeUndefined() + }) + }) }) diff --git a/test/utils.test.ts b/test/utils.test.ts index e1aeee6a..4a8a09e5 100644 --- a/test/utils.test.ts +++ b/test/utils.test.ts @@ -181,6 +181,35 @@ describe('mergeSchemaBranch', () => { expect(schema1.meta).toEqual({ nested: true }) }) + it('should keep a `false` property subschema when a branch adds a constraint', () => { + // A `false` subschema is unsatisfiable; per allOf semantics it must survive a + // sibling branch that constrains the same property, not be overwritten by it. + const schema1: Record = { properties: { m: false } } + mergeSchemaBranch(schema1, { properties: { m: { maximum: 12 } } }) + expect(schema1.properties.m).toBe(false) + }) + + it('should let a `false` branch subschema forbid a previously-typed property', () => { + const schema1: Record = { properties: { m: { maximum: 12 } } } + mergeSchemaBranch(schema1, { properties: { m: false } }) + expect(schema1.properties.m).toBe(false) + }) + + it('should keep a `false` property subschema forbidden across repeated branches', () => { + const schema1: Record = { properties: { m: false } } + mergeSchemaBranch(schema1, { properties: { m: { maximum: 12 } } }) + mergeSchemaBranch(schema1, { properties: { m: { minimum: 1 } } }) + expect(schema1.properties.m).toBe(false) + }) + + it('should let a branch relax a boolean keyword such as `additionalProperties`', () => { + // The forbidden-property guard applies to property subschemas, not to a boolean keyword + // whose `false` a branch may legitimately loosen. + const schema1: Record = { additionalProperties: false } + mergeSchemaBranch(schema1, { additionalProperties: true }) + expect(schema1.additionalProperties).toBe(true) + }) + it('should skip if/then/else properties', () => { const schema1: Record = { type: 'object' } mergeSchemaBranch(schema1, {