From 0a3aa4cb423e0b4e355836d06ff8bffcb85fb2ee Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Fri, 13 Feb 2026 13:45:53 +1300 Subject: [PATCH 1/3] fix(#650): allows multiple setvalue actions to reference the same element --- .changeset/bitter-seas-strive.md | 7 +++ .../actions-events/set-value-action.test.ts | 30 ++++++++++++- .../reactivity/createInstanceValueState.ts | 4 +- .../src/parse/model/ModelActionMap.ts | 45 +++++++++---------- 4 files changed, 59 insertions(+), 27 deletions(-) create mode 100644 .changeset/bitter-seas-strive.md diff --git a/.changeset/bitter-seas-strive.md b/.changeset/bitter-seas-strive.md new file mode 100644 index 000000000..3750f3474 --- /dev/null +++ b/.changeset/bitter-seas-strive.md @@ -0,0 +1,7 @@ +--- +'@getodk/xforms-engine': patch +'@getodk/scenario': patch +'@getodk/web-forms': patch +--- + +Allow for multiple setvalue actions to reference the same element diff --git a/packages/scenario/test/actions-events/set-value-action.test.ts b/packages/scenario/test/actions-events/set-value-action.test.ts index bb33e74f7..14c9bfb83 100644 --- a/packages/scenario/test/actions-events/set-value-action.test.ts +++ b/packages/scenario/test/actions-events/set-value-action.test.ts @@ -182,7 +182,7 @@ describe('setvalue action', () => { }); describe('region repeats', () => { - describe('[`setvalue`] source in repeat', () => { + describe('`setvalue` source in repeat', () => { // ported from: https://github.com/getodk/javarosa/blob/2dd8e15e9f3110a86f8d7d851efc98627ae5692e/src/test/java/org/javarosa/core/model/actions/SetValueActionTest.java#L251 it('updates destination in the same repeat instance', async () => { const scenario = await Scenario.init( @@ -728,4 +728,32 @@ describe('setvalue action', () => { expect(newInstance.attributeOf('/data/element', 'attr').getValue()).toBe('7'); }); }); + + it('allows multiple `setvalue` elements with the same `ref`', async () => { + const scenario = await Scenario.init( + 'Setvalue multiple', + html( + head( + title('Setvalue multiple'), + model( + mainInstance(t('data id="setvalue-multiple"', t('repeat id=""', t('source')))), + setvalueLiteral('odk-instance-first-load', '/data/repeat/source', 'first') + ) + ), + body( + repeat( + '/data/repeat', + input( + '/data/repeat/source', + setvalueLiteral('odk-new-repeat', '/data/repeat/source', 'second') + ) + ) + ) + ) + ); + expect(scenario.answerOf('/data/repeat[1]/source').getValue()).toBe('first'); + scenario.createNewRepeat('/data/repeat'); + expect(scenario.answerOf('/data/repeat[1]/source').getValue()).toBe('first'); + expect(scenario.answerOf('/data/repeat[2]/source').getValue()).toBe('second'); + }); }); diff --git a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts index 03085550b..b4dd131ae 100644 --- a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts +++ b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts @@ -346,8 +346,8 @@ export const createInstanceValueState = (context: ValueContext): InstanceValueSt createCalculation(context, setValue, calculate); } - const action = context.definition.model.actions.get(context.contextReference()); - if (action) { + const actions = context.definition.model.actions.get(context.contextReference()); + for (const action of actions) { dispatchAction(context, setValue, action); } diff --git a/packages/xforms-engine/src/parse/model/ModelActionMap.ts b/packages/xforms-engine/src/parse/model/ModelActionMap.ts index 418f415fe..5858dc81c 100644 --- a/packages/xforms-engine/src/parse/model/ModelActionMap.ts +++ b/packages/xforms-engine/src/parse/model/ModelActionMap.ts @@ -10,7 +10,7 @@ import type { ModelDefinition } from './ModelDefinition.ts'; const REPEAT_REGEX = /(\[[^\]]*\])/gm; -export class ModelActionMap extends Map { +export class ModelActionMap extends Map { static fromModel(model: ModelDefinition): ModelActionMap { return new this(model); } @@ -19,39 +19,36 @@ export class ModelActionMap extends Map { return ref.replace(REPEAT_REGEX, ''); } - private static processActions( + protected constructor(model: ModelDefinition) { + super(); + this.addAll(model, model.form.xformDOM.setValues, SET_VALUE_LOCAL_NAME); + this.addAll(model, model.form.xformDOM.setGeopoints, SET_GEOPOINT_LOCAL_NAME); + } + + override get(ref: string): ActionDefinition[] { + return super.get(ModelActionMap.getKey(ref)) ?? []; + } + + private addAll( model: ModelDefinition, elements: readonly DOMSetGeopointElement[] | readonly DOMSetValueElement[], type: string - ): Array<[string, ActionDefinition]> { - return elements.map((element) => { + ) { + for (const element of elements) { const action = new ActionDefinition(model, element); if (action.events.includes(XFORM_EVENT.odkNewRepeat)) { throw new Error(`Model contains "${type}" element with "odk-new-repeat" event`); } - const key = ModelActionMap.getKey(action.ref); - return [key, action]; - }); - } - - protected constructor(model: ModelDefinition) { - const entries: Array<[string, ActionDefinition]> = [ - ...ModelActionMap.processActions(model, model.form.xformDOM.setValues, SET_VALUE_LOCAL_NAME), - ...ModelActionMap.processActions( - model, - model.form.xformDOM.setGeopoints, - SET_GEOPOINT_LOCAL_NAME - ), - ]; - super(entries); - } - - override get(ref: string): ActionDefinition | undefined { - return super.get(ModelActionMap.getKey(ref)); + this.add(action); + } } add(action: ActionDefinition) { const key = ModelActionMap.getKey(action.ref); - this.set(key, action); + if (this.has(key)) { + this.get(key).push(action); + } else { + this.set(key, [action]); + } } } From 527a6ac8935f0446c575e8e107fbe67a8dbe90d6 Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Fri, 13 Feb 2026 14:32:42 +1300 Subject: [PATCH 2/3] check for undefined actions --- .../src/lib/reactivity/createInstanceValueState.ts | 6 ++++-- packages/xforms-engine/src/parse/model/ModelActionMap.ts | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts index b4dd131ae..2744d9adf 100644 --- a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts +++ b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts @@ -347,8 +347,10 @@ export const createInstanceValueState = (context: ValueContext): InstanceValueSt } const actions = context.definition.model.actions.get(context.contextReference()); - for (const action of actions) { - dispatchAction(context, setValue, action); + if (actions) { + for (const action of actions) { + dispatchAction(context, setValue, action); + } } return guardDownstreamReadonlyWrites(context, relevantValueState); diff --git a/packages/xforms-engine/src/parse/model/ModelActionMap.ts b/packages/xforms-engine/src/parse/model/ModelActionMap.ts index 5858dc81c..808bd4fec 100644 --- a/packages/xforms-engine/src/parse/model/ModelActionMap.ts +++ b/packages/xforms-engine/src/parse/model/ModelActionMap.ts @@ -25,8 +25,8 @@ export class ModelActionMap extends Map { this.addAll(model, model.form.xformDOM.setGeopoints, SET_GEOPOINT_LOCAL_NAME); } - override get(ref: string): ActionDefinition[] { - return super.get(ModelActionMap.getKey(ref)) ?? []; + override get(ref: string): ActionDefinition[] | undefined { + return super.get(ModelActionMap.getKey(ref)); } private addAll( @@ -46,7 +46,7 @@ export class ModelActionMap extends Map { add(action: ActionDefinition) { const key = ModelActionMap.getKey(action.ref); if (this.has(key)) { - this.get(key).push(action); + this.get(key)!.push(action); } else { this.set(key, [action]); } From 4fb35d96b4545ad0050258db97341f04a0b81283 Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Wed, 18 Feb 2026 09:10:15 +1300 Subject: [PATCH 3/3] review feedback --- .../src/lib/reactivity/createInstanceValueState.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts index 2744d9adf..26755dd76 100644 --- a/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts +++ b/packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts @@ -347,11 +347,7 @@ export const createInstanceValueState = (context: ValueContext): InstanceValueSt } const actions = context.definition.model.actions.get(context.contextReference()); - if (actions) { - for (const action of actions) { - dispatchAction(context, setValue, action); - } - } + actions?.forEach((action) => dispatchAction(context, setValue, action)); return guardDownstreamReadonlyWrites(context, relevantValueState); });