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, {