Fix (quick and dirty): client reactivity of parent node validation state

In theory, we could do something like this to simplify children state logic as well, at least until we have a better general solution to storing/serializing/materializing complex nested state like direct references to node objects. I’d prefer to keep it isolated for now and think about priority of a more holistic solution.

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[] {

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 {

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 {

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[] {

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

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[] {

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<AncestorNodeValidationState>;
+}
+
+/**
+ * @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<ShallowMutable<AncestorNodeValidationState>>({
+			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;
+			},
+		});
 	});
 };