diff --git a/packages/xforms-engine/src/instance/Group.ts b/packages/xforms-engine/src/instance/Group.ts index 6fb1fcc5..44b08f91 100644 --- a/packages/xforms-engine/src/instance/Group.ts +++ b/packages/xforms-engine/src/instance/Group.ts @@ -54,6 +54,10 @@ export class Group this.childrenState = childrenState; + const sharedStateOptions = { + clientStateFactory: this.engineConfig.stateFactory, + }; + const state = createSharedNodeState( this.scope, { @@ -68,9 +72,7 @@ export class Group valueOptions: null, value: null, }, - { - clientStateFactory: this.engineConfig.stateFactory, - } + sharedStateOptions ); this.state = state; @@ -82,7 +84,7 @@ export class Group ); childrenState.setChildren(buildChildren(this)); - this.validationState = createAggregatedViolations(this); + this.validationState = createAggregatedViolations(this, sharedStateOptions); } getChildren(): readonly GeneralChildNode[] { diff --git a/packages/xforms-engine/src/instance/RepeatInstance.ts b/packages/xforms-engine/src/instance/RepeatInstance.ts index 30c8edf9..2c74dab5 100644 --- a/packages/xforms-engine/src/instance/RepeatInstance.ts +++ b/packages/xforms-engine/src/instance/RepeatInstance.ts @@ -119,6 +119,10 @@ export class RepeatInstance this.currentIndex = currentIndex; + const sharedStateOptions = { + clientStateFactory: this.engineConfig.stateFactory, + }; + const state = createSharedNodeState( this.scope, { @@ -134,9 +138,7 @@ export class RepeatInstance valueOptions: null, value: null, }, - { - clientStateFactory: this.engineConfig.stateFactory, - } + sharedStateOptions ); this.state = state; @@ -169,7 +171,7 @@ export class RepeatInstance }); childrenState.setChildren(buildChildren(this)); - this.validationState = createAggregatedViolations(this); + this.validationState = createAggregatedViolations(this, sharedStateOptions); } protected override initializeContextNode(parentContextNode: Element, nodeName: string): Element { diff --git a/packages/xforms-engine/src/instance/RepeatRange.ts b/packages/xforms-engine/src/instance/RepeatRange.ts index 3db3c0aa..24fe6071 100644 --- a/packages/xforms-engine/src/instance/RepeatRange.ts +++ b/packages/xforms-engine/src/instance/RepeatRange.ts @@ -199,6 +199,10 @@ export class RepeatRange definition.bind.relevant ); + const sharedStateOptions = { + clientStateFactory: this.engineConfig.stateFactory, + }; + const state = createSharedNodeState( this.scope, { @@ -213,9 +217,7 @@ export class RepeatRange valueOptions: null, value: null, }, - { - clientStateFactory: this.engineConfig.stateFactory, - } + sharedStateOptions ); this.state = state; @@ -231,7 +233,7 @@ export class RepeatRange this.addInstances(afterIndex, 1, instanceDefinition); }); - this.validationState = createAggregatedViolations(this); + this.validationState = createAggregatedViolations(this, sharedStateOptions); } private getLastIndex(): number { diff --git a/packages/xforms-engine/src/instance/Root.ts b/packages/xforms-engine/src/instance/Root.ts index 6702c295..127bb067 100644 --- a/packages/xforms-engine/src/instance/Root.ts +++ b/packages/xforms-engine/src/instance/Root.ts @@ -155,6 +155,10 @@ export class Root const evaluator = instanceDOM.primaryInstanceEvaluator; const { translations } = evaluator; const { defaultLanguage, languages } = getInitialLanguageState(translations); + const sharedStateOptions = { + clientStateFactory: this.engineConfig.stateFactory, + }; + const state = createSharedNodeState( this.scope, { @@ -169,9 +173,7 @@ export class Root value: null, children: childrenState.childIds, }, - { - clientStateFactory: engineConfig.stateFactory, - } + sharedStateOptions ); this.state = state; @@ -192,7 +194,7 @@ export class Root this.languages = languages; childrenState.setChildren(buildChildren(this)); - this.validationState = createAggregatedViolations(this); + this.validationState = createAggregatedViolations(this, sharedStateOptions); } getChildren(): readonly GeneralChildNode[] { diff --git a/packages/xforms-engine/src/instance/StringField.ts b/packages/xforms-engine/src/instance/StringField.ts index 008234a6..cb8ee4e9 100644 --- a/packages/xforms-engine/src/instance/StringField.ts +++ b/packages/xforms-engine/src/instance/StringField.ts @@ -98,12 +98,7 @@ export class StringField } getViolation(): AnyViolation | null { - // Read engine state to ensure reactivity in engine, Solid-based clients - this.validation.engineState.violation; - - // Read/return client state to ensure client reactivity, regardless of - // client's reactive implementation - return this.validationState.violation; + return this.validation.engineState.violation; } // ValidationContext diff --git a/packages/xforms-engine/src/instance/Subtree.ts b/packages/xforms-engine/src/instance/Subtree.ts index b6d665f9..ea05774a 100644 --- a/packages/xforms-engine/src/instance/Subtree.ts +++ b/packages/xforms-engine/src/instance/Subtree.ts @@ -49,6 +49,10 @@ export class Subtree this.childrenState = childrenState; + const sharedStateOptions = { + clientStateFactory: this.engineConfig.stateFactory, + }; + const state = createSharedNodeState( this.scope, { @@ -63,9 +67,7 @@ export class Subtree valueOptions: null, value: null, }, - { - clientStateFactory: this.engineConfig.stateFactory, - } + sharedStateOptions ); this.state = state; @@ -77,7 +79,7 @@ export class Subtree ); childrenState.setChildren(buildChildren(this)); - this.validationState = createAggregatedViolations(this); + this.validationState = createAggregatedViolations(this, sharedStateOptions); } getChildren(): readonly GeneralChildNode[] { diff --git a/packages/xforms-engine/src/lib/reactivity/validation/createAggregatedViolations.ts b/packages/xforms-engine/src/lib/reactivity/validation/createAggregatedViolations.ts index 3b125fb7..2db84c14 100644 --- a/packages/xforms-engine/src/lib/reactivity/validation/createAggregatedViolations.ts +++ b/packages/xforms-engine/src/lib/reactivity/validation/createAggregatedViolations.ts @@ -1,4 +1,6 @@ -import { createMemo } from 'solid-js'; +import type { ShallowMutable } from '@getodk/common/types/helpers.js'; +import { createComputed, createMemo, on } from 'solid-js'; +import type { OpaqueReactiveObjectFactory } from '../../../client/OpaqueReactiveObjectFactory.ts'; import type { AncestorNodeValidationState, DescendantNodeViolationReference, @@ -46,16 +48,58 @@ const collectViolationReferences = ( }); }; -export const createAggregatedViolations = (context: AnyParentNode): AncestorNodeValidationState => { +interface AggregatedViolationsOptions { + readonly clientStateFactory: OpaqueReactiveObjectFactory; +} + +/** + * @todo This implementation is intentionally naive! Since we anticipate the + * possibility of making computing validation state lazier, aggregated state is + * a good candidate for where we might start. The solution here is intended to + * unblock client reactivity without expanding or entrenching any particular + * approach to storing/serializing/materializing complex shared state object + * values (i.e. {@link DescendantNodeViolationReference.node}). + */ +export const createAggregatedViolations = ( + context: AnyParentNode, + options: AggregatedViolationsOptions +): AncestorNodeValidationState => { return context.scope.runTask(() => { + const clientState = options.clientStateFactory>({ + violations: [] as readonly DescendantNodeViolationReference[], + }); + const violations = createMemo(() => { return collectViolationReferences(context); }); - return { - get violations() { - return violations(); + createComputed( + on(violations, () => { + // Acknowledging this is a naive implementation: the intent here is that + // it doesn't really matter **what value** is written, only that a write + // has occurred. The corresponding read in the Proxy below is sufficient + // to register read-based subscriptions (at least as tested in Vue). + clientState.violations = []; + }) + ); + + return new Proxy(clientState, { + get(_, property, target) { + if (property === 'violations') { + // This check doesn't matter, we just need to read the state. + if (clientState.violations.length === 0) { + return violations(); + } + } + + // Pass through any other reads, which may vary by client reactivity. + // Many Proxy-based mutable store solutions attach one or more internal + // Symbol properties that they will reference for... reasons. + return Reflect.get(clientState, property, target) as unknown; }, - }; + set() { + return false; + }, + }); }); };