feat(#122): add support for jr:preload #571

garethbowen · 2025-11-27T21:44:44Z

Closes #122

I have verified this PR works in these browsers (latest versions):

What else has been done to verify that this works as intended?

Testing.

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Introduces new functionality.

Do we need any specific form for testing your changes? If so, please attach one.

From this PR: packages/common/src/fixtures/test-javarosa/resources/preload.xml

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa">
    <h:head>
        <h:title>jr:preload</h:title>
        <model>
            <instance>
                <data id="preload" version="1">
                    <today/>
                    <start/>
                    <end/>
                    <deviceid/>
                    <phonenumber/>
                    <email/>
                    <username/>
                    <meta>
                        <instanceID/>
                    </meta>
                </data>
            </instance>
            <bind jr:preload="date" jr:preloadParams="today" nodeset="/data/today" type="date" readonly="true()"/>
            <bind jr:preload="timestamp" jr:preloadParams="start" nodeset="/data/start" type="dateTime" readonly="true()"/>
            <bind jr:preload="timestamp" jr:preloadParams="end" nodeset="/data/end" type="dateTime" readonly="true()"/>
            <bind jr:preload="property" jr:preloadParams="deviceid" nodeset="/data/deviceid" type="string" readonly="true()"/>
            <bind jr:preload="property" jr:preloadParams="phonenumber" nodeset="/data/phonenumber" type="string" readonly="true()"/>
            <bind jr:preload="property" jr:preloadParams="email" nodeset="/data/email" type="string" readonly="true()"/>
            <bind jr:preload="property" jr:preloadParams="username" nodeset="/data/username" type="string" readonly="true()"/>
            <bind jr:preload="uid" nodeset="/data/meta/instanceID" type="string" readonly="true()"/>
        </model>
    </h:head>
    <h:body>
        <input ref="/data/today">
            <label>today</label>
        </input>
        <input ref="/data/start">
            <label>start</label>
        </input>
        <input ref="/data/end">
            <label>end</label>
        </input>
        <input ref="/data/deviceid">
            <label>deviceid</label>
        </input>
        <input ref="/data/phonenumber">
            <label>phonenumber</label>
        </input>
        <input ref="/data/email">
            <label>email</label>
        </input>
        <input ref="/data/username">
            <label>username</label>
        </input>
        <input ref="/data/meta/instanceID">
            <label>instanceID</label>
        </input>
    </h:body>
</h:html>

What's changed

When loading the form, check for bind preloads, and set the value as appropriate.
When preparing for submission, check for end binds and set the value.

changeset-bot · 2025-11-27T21:44:47Z

🦋 Changeset detected

Latest commit: bc3db40

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@getodk/xforms-engine	Minor
@getodk/web-forms	Minor
@getodk/scenario	Minor
@getodk/common	Minor
@getodk/xpath	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

garethbowen · 2025-11-27T21:45:39Z

packages/web-forms/e2e/page-objects/controls/InputControl.ts

-		const input = container.locator('input');
+		const input = this.page.locator(
+			`.question-container:has(.control-text label:text-is("${label}")) input`
+		);


This is within the margin of error but in my testing speeds up the suite by around 4%

garethbowen · 2025-11-27T21:48:37Z

packages/web-forms/src/components/OdkWebForm.vue

 export interface OdkWebFormsProps {
 	readonly formXml: string;
 	readonly fetchFormAttachment: FetchFormAttachment;
+	readonly preloadProperties?: PreloadProperties;


You can now optionally pass in properties in to the vue component, specifically the users email, phone number, and username. Additionally you can pass in the deviceid - if you don't then one will be generated for you.

garethbowen · 2025-11-27T21:48:55Z

packages/web-forms/src/demo/FormPreview.vue

+	email: 'fake@fake.fake',
+	phoneNumber: '+1235556789',
+	username: 'nousername',
+};


I needed something for the e2e testing to work with.

garethbowen · 2025-11-27T21:50:56Z

packages/web-forms/src/lib/init/load-form-state.ts

+		return id;
+	}
+	const deviceId = DEVICE_ID_PREFIX + crypto.randomUUID();
+	localStorage.setItem(DEVICE_ID_KEY, deviceId);


By default this will get generated and stored in the browser. However it won't get used unless you specify the bind in the xform. I think this is acceptable from a tracking perspective, because the part that goes to the server is opt-in.

This seems fine for all general cases. I want to double-check the following:

Should it warn that the device ID wasn't provided? It might be appreciated if form designers could be notified before publishing and distributing the form.

If the device ID is provided later, and there are already submissions from the previous weeks using the value from LocalStorage, which takes priority? I assume the new provided device ID, even if it doesn't match the previous submissions.

Is it considered personal/private info that we need to inform users that it's been stored in the browser? I guess not, since it is generated in this case, but I've seen other apps that consider it personal info.

Can someone inject something malicious since it's in the localStorage? I wonder if Alex can crack it 😁

Should it warn that the device ID wasn't provided?

I'm not following what you mean here. Do you mean at the form preview stage? Do you mean if the ID wasn't provided by the client when calling the vue component, or wasn't found in local storage?

The way it works right now is on the first visit a new ID will be generated and stored. This default behaviour can be overwritten by the client including the webforms vue component by providing their own device ID (which could be a hardcoded string if they want to disable tracking altogether), which could be generated in their own way. Then if a form is loaded that uses the deviceid preload parameter that ID will be bound into the model.

If the device ID is provided later, and there are already submissions from the previous weeks using the value from LocalStorage, which takes priority? I assume the new provided device ID, even if it doesn't match the previous submissions.

Correct. If an ID is provided from the client then this code is never executed. This would only happen if the client invoking the vue component was updated to provide the value.

Is it considered personal/private info that we need to inform users that it's been stored in the browser?

I think it should be treated the same as a cookie (eg: auth cookie in our case). This is probably up to the web application to handle to meet their local specific needs/legal requirements. However, maybe this is a reason to make storage opt-in rather than opt-out. Right now it's a little sneaky and someone using the vue component probably wouldn't expect it to store anything, so it makes it hard for the calling app to handle their legal requirements. I'll start a discussion in Slack to resolve this.

Can someone inject something malicious since it's in the localStorage?

Good thinking! I've played around with it myself and it's escaping correctly for me. There are two possible vectors I can think of...

Put something in localstorage which does something when displayed in the form. In this case the value is escaped by vue when rendering. When I tried it characters were escaped as expected.

Put something in localstorage which does something to the submitted xml. In this case we call escapeXMLText during serialization. In my testing this was also escaped as expected.

garethbowen · 2025-11-27T21:54:44Z

packages/xforms-engine/src/parse/model/ModelDefinition.ts


-		return rest;
+	triggerXformsRevalidateListeners() {
+		this.listeners.forEach((listener: XformsRevalidateListener) => listener());


Using listeners in this way feels slightly awkward. An alternative would be to iterate through the binds looking for revalidate actions which would be cleaner but be a performance hit. I'm open to suggestions here...

My thinking process:

Yes, it feels awkward at first, especially since these listeners aren't yet using anything directly from the model, like this.form, this.nodes, etc (looking at L113 of createInstanceValueState.ts). But it's a convenient way to carry those callbacks to the moment the payload is prepared (prepareInstancePayload.ts).

When I see "listeners" it makes me wonder if at some point it should be unsubscribed, call it trauma from using JS Observable libraries for a long time, haha. Different from when I see "recalculate", "recompute" in Solid's runTasks. Here, it seems we don't need to "unsubscribe" because they are in the model, which is convenient.

You can consider a rename, since these "listeners" run just before submission, right?

And it's not like this model could keep the preloads only private preloads: AnyBindPreloadDefinition[] and leave the rest to prepareInstancePayload.ts because there's no easy access to the "context" and "setValue" 🤔

I renamed them to make it more clear.

Aside from our shared awkwardness it sounds like we have no better ideas, so I'll leave it as is for now.

Yes, I didn't have a better idea than adding the callback to the model. Using the map is a nice improvement!

latin-panda

Looks good! I have some questions below

Would you like to release once this is merged? I think Central will code freeze this week for regression testing of .4

latin-panda · 2025-11-30T16:52:31Z

packages/web-forms/src/lib/init/load-form-state.ts

+		return id;
+	}
+	const deviceId = DEVICE_ID_PREFIX + crypto.randomUUID();
+	localStorage.setItem(DEVICE_ID_KEY, deviceId);


This seems fine for all general cases. I want to double-check the following:

Should it warn that the device ID wasn't provided? It might be appreciated if form designers could be notified before publishing and distributing the form.

If the device ID is provided later, and there are already submissions from the previous weeks using the value from LocalStorage, which takes priority? I assume the new provided device ID, even if it doesn't match the previous submissions.

Is it considered personal/private info that we need to inform users that it's been stored in the browser? I guess not, since it is generated in this case, but I've seen other apps that consider it personal info.

Can someone inject something malicious since it's in the localStorage? I wonder if Alex can crack it 😁

packages/xforms-engine/src/parse/model/Event.ts

packages/xforms-engine/src/lib/reactivity/createInstanceValueState.ts

latin-panda · 2025-11-30T17:32:35Z

packages/xforms-engine/src/parse/model/ModelDefinition.ts


-		return rest;
+	triggerXformsRevalidateListeners() {
+		this.listeners.forEach((listener: XformsRevalidateListener) => listener());


My thinking process:

Yes, it feels awkward at first, especially since these listeners aren't yet using anything directly from the model, like this.form, this.nodes, etc (looking at L113 of createInstanceValueState.ts). But it's a convenient way to carry those callbacks to the moment the payload is prepared (prepareInstancePayload.ts).

When I see "listeners" it makes me wonder if at some point it should be unsubscribed, call it trauma from using JS Observable libraries for a long time, haha. Different from when I see "recalculate", "recompute" in Solid's runTasks. Here, it seems we don't need to "unsubscribe" because they are in the model, which is convenient.

You can consider a rename, since these "listeners" run just before submission, right?

And it's not like this model could keep the preloads only private preloads: AnyBindPreloadDefinition[] and leave the rest to prepareInstancePayload.ts because there's no easy access to the "context" and "setValue" 🤔

latin-panda

Looks good! Waiting for others' feedback on local storage notification, but approving now if we proceed as is.

garethbowen commented Nov 27, 2025

View reviewed changes

garethbowen requested a review from latin-panda November 27, 2025 21:55

latin-panda reviewed Nov 30, 2025

View reviewed changes

garethbowen added 15 commits December 1, 2025 20:11

first steps of jr preload

b090314

first stab at passing property preloads

b0a6797

fix mock primary instance

e22d097

improve typing

b6960bb

dates should include client tz

3b71aa8

fix tz regex for my eastern friends

97514b9

make regex a little more permissive

6a4c51c

extract datetime format functions

7a25550

generate and store deviceid

6e8e1c1

add testing

bd777db

prefer xpath fns to js ones

03ed57e

clean up getvalue

d870111

review feedback

83a4c4d

review feedback

19da6b5

rebuild yarn.lock

bc3db40

garethbowen force-pushed the 122-jr-preload branch from d784f3b to bc3db40 Compare December 1, 2025 07:14

latin-panda approved these changes Dec 1, 2025

View reviewed changes

feat(#122): add support for jr:preload #571

Are you sure you want to change the base?

feat(#122): add support for jr:preload #571

Conversation

garethbowen commented Nov 27, 2025

I have verified this PR works in these browsers (latest versions):

What else has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Do we need any specific form for testing your changes? If so, please attach one.

What's changed

Uh oh!

changeset-bot bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latin-panda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latin-panda left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Nov 27, 2025 •

edited

Loading