diff --git a/src/__tests__/module-engine/validateNodeProps.test.ts b/src/__tests__/module-engine/validateNodeProps.test.ts index 2f98bef5e..31c1481db 100644 --- a/src/__tests__/module-engine/validateNodeProps.test.ts +++ b/src/__tests__/module-engine/validateNodeProps.test.ts @@ -12,6 +12,8 @@ * rawProps for them); Ref/recursive/cyclic schemas degrade safely to * the slow path. * (f) Every base module's propsSchema is fast-path eligible. + * (g) Per-prop repair: when one prop cannot be coerced, only that prop + * takes the module default; every other authored prop survives. */ import { describe, it, expect } from 'bun:test' @@ -86,8 +88,7 @@ describe('validateNodeProps — (a) coerce to schema defaults', () => { expect(result.visible).toBe(false) }) - it('falls back to module defaults when coercion fails catastrophically', () => { - // Provide a deeply invalid value that Value.Parse cannot recover. + it('falls back to the module default for a prop coercion cannot fix', () => { // We force a failure by using a schema whose type can't be coerced. const strictSchema = Type.Object({ id: Type.String({ pattern: '^[a-z]+$', default: 'fallback' }), @@ -98,11 +99,121 @@ describe('validateNodeProps — (a) coerce to schema defaults', () => { }) // "123" fails the /^[a-z]+$/ pattern — coercion cannot fix it. const result = validateNodeProps(strictDef, { id: '123' }) - // Should fall back to defaults expect(result.id).toBe('fallback') }) }) +// --------------------------------------------------------------------------- +// (g) Per-prop repair — one bad prop never resets the whole node +// --------------------------------------------------------------------------- + +describe('validateNodeProps — (g) per-prop repair', () => { + const RepairSchema = Type.Object({ + href: Type.String({ default: '#' }), + text: Type.String({ default: 'Click here' }), + target: Type.Union([Type.Literal('_self'), Type.Literal('_blank')], { default: '_self' }), + htmlAttributes: Type.Record(Type.String(), Type.String(), { default: {} }), + note: Type.Optional(Type.String()), + }) + const def = stubDef({ + propsSchema: RepairSchema, + defaults: Value.Create(RepairSchema) as Record, + }) + + it('a bad enum value takes its default while sibling props keep their authored values', () => { + const result = validateNodeProps(def, { + href: '/contact/', + text: 'Contact us', + target: '', + htmlAttributes: { id: 'cta' }, + }) + expect(result.target).toBe('_self') + expect(result.href).toBe('/contact/') + expect(result.text).toBe('Contact us') + expect(result.htmlAttributes).toEqual({ id: 'cta' }) + }) + + it('a bad nested object resets only that prop', () => { + const result = validateNodeProps(def, { + href: '/contact/', + text: 'Contact us', + target: '_blank', + htmlAttributes: { id: { nested: true } }, + }) + expect(result.htmlAttributes).toEqual({}) + expect(result.href).toBe('/contact/') + expect(result.text).toBe('Contact us') + expect(result.target).toBe('_blank') + }) + + it('still coerces and default-fills the props that can be repaired', () => { + const CountSchema = Type.Object({ + count: Type.Number({ default: 1 }), + mode: Type.Union([Type.Literal('a'), Type.Literal('b')], { default: 'a' }), + label: Type.String({ default: 'x' }), + }) + const countDef = stubDef({ + propsSchema: CountSchema, + defaults: Value.Create(CountSchema) as Record, + }) + const result = validateNodeProps(countDef, { count: '7', mode: 'zzz' }) + expect(result.count).toBe(7) + expect(result.mode).toBe('a') + expect(result.label).toBe('x') + }) + + it('an absent optional prop stays absent during repair', () => { + const result = validateNodeProps(def, { href: '/x', text: 'X', target: 'nope' }) + expect('note' in result).toBe(false) + expect(result.target).toBe('_self') + }) + + it('an absent optional prop with a default is filled during repair', () => { + const OptSchema = Type.Object({ + mode: Type.Union([Type.Literal('a'), Type.Literal('b')], { default: 'a' }), + opt: Type.Optional(Type.String({ default: 'OPT' })), + }) + const optDef = stubDef({ propsSchema: OptSchema, defaults: { mode: 'a' } }) + expect(validateNodeProps(optDef, { mode: 'zzz' }).opt).toBe('OPT') + }) + + it('repair does not throw when a plugin module ships non-object defaults', () => { + const badDef = stubDef({ + propsSchema: RepairSchema, + defaults: undefined as unknown as Record, + }) + expect(() => validateNodeProps(badDef, { href: '/x', text: 'X', target: 'nope' })).not.toThrow() + }) + + it('injected unknown keys survive repair', () => { + const media = { img: { url: 'https://example.com/a.jpg' } } + const result = validateNodeProps(def, { target: 'nope', _resolvedMediaByKey: media }) + expect(result._resolvedMediaByKey).toBe(media) + }) + + it('base.link: an unsupported target keeps href and text at publish time', () => { + const link = registry.getOrThrow('base.link') + const result = validateNodeProps(link, { + href: '/contact/', + text: 'Contact us', + target: '', + htmlAttributes: {}, + }) + expect(result.target).toBe('_self') + expect(result.href).toBe('/contact/') + expect(result.text).toBe('Contact us') + }) + + it('a non-object props schema still falls back to the module defaults wholesale', () => { + const arrDef = stubDef({ + propsSchema: Type.Array(Type.Number()), + defaults: { items: [1] }, + }) + const result = validateNodeProps(arrDef, { items: ['x'] }) + expect(result.items).toEqual([1]) + }) +}) + // --------------------------------------------------------------------------- // (b) Unknown injected fields survive validation // --------------------------------------------------------------------------- diff --git a/src/core/module-engine/validateNodeProps.ts b/src/core/module-engine/validateNodeProps.ts index 820fe6d4b..3e7a51044 100644 --- a/src/core/module-engine/validateNodeProps.ts +++ b/src/core/module-engine/validateNodeProps.ts @@ -24,6 +24,10 @@ * exactly as before, for non-conforming values and for schemas where * Check-pass does not imply Parse-identity. * + * 3. REPAIR — when the slow path throws, each declared prop is parsed on + * its own and only the props that still fail take the module default + * (see `repairProps`). One unsupported value never resets the node. + * * Design constraints: * - SOFT boundary — exceptions from coercion are caught; never bubbles. * - Unknown/injected keys survive — the fast path returns them untouched on @@ -181,8 +185,11 @@ function fastPathEligible(schema: TSchema): boolean { * - Schema present, coercion succeeds → `{ ...rawProps, ...cleanedProps }`. * Known props are coerced/defaulted by Value.Parse; unknown keys from * rawProps survive untouched. - * - Schema present, coercion fails → `{ ...rawProps, ...def.defaults }`. - * Falls back to module defaults for known keys, unknown keys still survive. + * - Schema present, coercion fails → `{ ...rawProps, ...repaired }` where + * each declared prop is parsed individually and only the props that + * cannot be coerced fall back to the module default for that key, when + * one is declared. + * Unknown keys still survive. */ export function validateNodeProps( def: AnyModuleDefinition, @@ -203,9 +210,54 @@ export function validateNodeProps( const cleaned = parseValue(def.propsSchema, rawProps) as Record return { ...rawProps, ...cleaned } } catch (_err) { - // Value.Parse threw — the input is unrecoverable for this schema even - // after applying defaults and type coercions. Fall back to the module's - // declared defaults, while still preserving any injected unknown keys. - return { ...rawProps, ...def.defaults } + // Value.Parse threw — at least one prop cannot be coerced into shape. + // Repair per prop so one bad value costs only that value, not the + // node's whole authored content; injected unknown keys still survive. + return { ...rawProps, ...repairProps(def, rawProps) } + } +} + +/** + * Tier 3 — per-prop repair for an object schema whose whole-value Parse + * failed. Every declared prop is parsed on its own (Default + Convert + + * Check for that leaf); a prop that still fails takes the module's declared + * default for that key. Props that pass stay exactly as authored, so a link + * with an unsupported `target` keeps its `href` and `text`, and a stale + * enum on one field does not wipe the rest of the node. + * + * A non-object schema has no per-prop granularity and keeps the old + * behaviour: the module defaults replace the value wholesale. So does a + * schema without TypeBox's `Kind` symbol (for example one rebuilt from + * JSON), because Value.Parse cannot read it. + */ +function repairProps( + def: AnyModuleDefinition, + rawProps: Record, +): Record { + const schema = def.propsSchema as TSchema + const properties: unknown = schema.properties + if (schema[Kind] !== 'Object' || !isSchemaObject(properties)) { + return { ...def.defaults } + } + + // Browser-loaded plugin packs pass `defaults` through unvalidated; this + // boundary never throws, so treat a non-object as "no defaults". + const defaults: Record = + typeof def.defaults === 'object' && def.defaults !== null ? def.defaults : {} + const repaired: Record = {} + for (const [key, propSchema] of Object.entries(properties)) { + if (!isSchemaObject(propSchema)) continue + const value = rawProps[key] + // An absent optional prop without a default is valid as-is; Parse on the + // bare leaf would reject `undefined` because the Optional modifier only + // means something inside the parent object. One with a default falls + // through so it is filled, as the successful slow path does. + if (value === undefined && OptionalKind in propSchema && !('default' in propSchema)) continue + try { + repaired[key] = parseValue(propSchema, value) + } catch (_err) { + if (key in defaults) repaired[key] = defaults[key] + } } + return repaired }