preserve the form data so when we update the additionalErrors the dat… by kchobantonov · Pull Request #2478 · eclipsesource/jsonforms

kchobantonov · 2025-08-12T17:35:57Z

…a is not reset to the initial state

netlify · 2025-08-12T17:36:02Z

✅ Deploy Preview for jsonforms-examples ready!

Name	Link
🔨 Latest commit	`ade1fa2`
🔍 Latest deploy log	https://app.netlify.com/projects/jsonforms-examples/deploys/6a70f8f763c8d200084210e2
😎 Deploy Preview	https://deploy-preview-2478--jsonforms-examples.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

kchobantonov · 2025-08-12T17:40:23Z

@sdirix please review, to check the issue in the current master branch just apply the change for ExampleView.vue and then try to test the additional-errors example, e.g. change the form once it is loaded and then click on the "Add Additonal Error" button and observe that the form data is reset, then check the https://deploy-preview-2478--jsonforms-examples.netlify.app/vue-vuetify/#additional-errors

coveralls · 2025-08-13T18:17:39Z

coverage: 84.29% (+0.004%) from 84.286% — kchobantonov:preserve-edit-data into eclipsesource:master

sdirix · 2025-08-13T18:24:47Z

So the error occurs only if the user of JSON Forms:

hands over new additionalErrors to JSON Forms component
does not hand over an updated state of the the data

I don't think that this is a valid use case. Either the props of the JSON Forms component are "frozen" and the management is done by JSON Forms, or the props are always live. The error case is a weird mix which is not really valid.

The same issue will occur in Angular and React if only parts of the props are updated. Either they should never be updated by the user, i.e. uncontrolled variant, or they should all be updated, i.e. controlled variant.

kchobantonov · 2025-08-15T05:18:31Z

I understand the distinction between controlled and uncontrolled usage, but in practice consumers of JSON Forms may mix these patterns—intentionally or unintentionally—especially in larger applications with multiple state sources.
From a robustness standpoint, the component should ideally handle all variants, including partial updates.

Even if this “weird mix” is not the recommended pattern, supporting it makes JSON Forms more fault-tolerant and easier to integrate in real-world scenarios where the state flow is not always perfectly aligned with controlled/uncontrolled paradigms.

…the data is changed

…imple type like number, string, array and etc.

kchobantonov · 2025-08-24T01:13:42Z

@sdirix @lucas-koehler please also check the update where the generated schema and uischema are properly regenerated when the data is changes - please check this example dynamic when if you change the data to a number and save then the ui will be updated accordingly - in previous versions that was not true.

… returning null

…to non null uischema in JsonForms

kchobantonov · 2025-09-27T18:28:08Z

@sdirix please review

kchobantonov · 2025-11-10T15:34:33Z

@sdirix @lucas-koehler please review

lucas-koehler

Hi @kchobantonov ,
Thanks for the updates! I have some comments inline unrelated to the setting of this.dataToUse = newEvent.data;.

I am still a bit concerned about this:

It makes the data propagation less intuitive:
- In the controlled case, the data is set to dataToUse again after it has already been updated. Granted, this does not trigger an additional invocation of coreDataToUpdate().
- In the uncontrolled case, this triggers coreDataToUpdate an additional time without it being needed. This has a possible performance penalty because it invokes the middleware again
- In the mixed case, it triggers the coreDataToUpdate like in the uncontrolled case. From a data point of view that should also not be necessary because the data is uncontrolled. Granted, this is acceptable considering the issues that this tries to fix.

As this does make the state handling less intuitive and lowers performance in uncontrolled mode as far as I can tell, I am leaning towards not introducing supporting this mixed case.

Do you have a concrete use case in mind where this is necessary? For a diffuse robustness increase, I prefer to not introduce this because consumers can always control the data in the component rendering the form. Even if the state handling is more complex for other properties.
We can of course add this to the documentation though.

kchobantonov · 2025-12-07T18:36:03Z

Hi @kchobantonov , Thanks for the updates! I have some comments inline unrelated to the setting of this.dataToUse = newEvent.data;.

I am still a bit concerned about this:

It makes the data propagation less intuitive:

In the controlled case, the data is set to dataToUse again after it has already been updated. Granted, this does not trigger an additional invocation of coreDataToUpdate().

In the uncontrolled case, this triggers coreDataToUpdate an additional time without it being needed. This has a possible performance penalty because it invokes the middleware again

In the mixed case, it triggers the coreDataToUpdate like in the uncontrolled case. From a data point of view that should also not be necessary because the data is uncontrolled. Granted, this is acceptable considering the issues that this tries to fix.

As this does make the state handling less intuitive and lowers performance in uncontrolled mode as far as I can tell, I am leaning towards not introducing supporting this mixed case.

Do you have a concrete use case in mind where this is necessary? For a diffuse robustness increase, I prefer to not introduce this because consumers can always control the data in the component rendering the form. Even if the state handling is more complex for other properties. We can of course add this to the documentation though.

Yes I have a specific case in my camunda-jsonforms-plugin project where when the form is submitted then if the backend determines that some of the fields is not valid because of some runtime constraints then I'm returning back to the UI additionalErrors so those can be rendered on the UI to represent the server side validated fields but when I set the additionalErrors that triggers the setting of the forms data and etc. which defacto reverts my changed data to whatever I had on the first load which is definitely not what I wanted since that will not display the data in the form as it was before submitting that. It is given that I can by pass that (in the example project even I do capture the data and preserve that) but since that looks like a bug to me to be honest I wanted to see that in the core itself instead of me handling that in multiple places like the vuetify3 demo app in the jsonforms project, in my camunda-jsonforms-plugin in the jsonforms-vuetify-webcomponent project and so on.

kchobantonov · 2025-12-07T18:45:19Z

@lucas-koehler thanks for the review - please check my comments

sdirix · 2025-12-12T12:11:25Z

I think we just have a clash of expectations here.

@kchobantonov is treating data like a defaultValue in React. Handing over an initial value and never expected to update it. We, the JSON Forms team, treat data like a controlled prop which needs to be updated.

This is additionally complicated by the fact that data is optional. The user could expect that if they never handed in data and only update the additionalErrors, that they still do not need to update their data. But this is not the case currently.

I think it would be good to clearly differentiate the two behaviors and therefore expectations. For this we can introduce two new props, modelValue which is the usual name in Vue 3 with v-model support, and defaultValue for the "hand over once and never update" case. Accordingly we will then behave within the JSON Forms component. We keep data as it is currently for backward compatibility, i.e. treat it the same way as modelValue.

Alternatively we could also not create a new modelValue prop but just offer an update:data event for custom v-model:data support. In fact that's even better to avoid introducing a redundant variable.

Does this make sense for everybody? @lucas-koehler What do you think? @kchobantonov would you be willing to implement this?

lucas-koehler · 2025-12-12T13:31:18Z

@sdirix Thanks for the explanation. I agree with your sentiment. Both suggested solutions are fine with me but I prefer the second one to keep the API of the JSON Forms component concise. I.e.:

Alternatively we could also not create a new modelValue prop but just offer an update:data event for custom v-model:data support. In fact that's even better to avoid introducing a redundant variable.

kchobantonov · 2025-12-24T17:13:17Z

so what is the goal here then ? introduce another event for changing data only

Yes, I also don’t think we need to introduce new props—or events, for that matter.
The example I outlined can indeed be interpreted as using the value only as an initial input for the form, without updating it from the outside afterward. However, that does not cover the full range of use cases. In my case, there are scenarios where the form data is modified as a result of external actions, typically triggered by clicking a button, and those updates need to be reflected in the form.
The issues I’m facing with the current implementation are the following:

Updating additionalErrors causes a full internal update with the previously set data (overriding the data that was updated meanwhile)
When I update additionalErrors, I do not expect this to trigger that behaviour. In particular, the data currently entered in the form should be preserved.
I do have a workaround that avoids changing the JsonForms component itself, but that solution would require duplicating the same logic across multiple projects that I have. Conceptually, I don’t think that changing a property which is not directly related to the data should result in the data being reset.

This is the case we mostly discussed here in the PR. So when you have the use case that you update data externally, you would use v-model:data and then it's always up to date. Changing additionalErrors additionally should work without an issue then.

Schema and UI model are not recomputed when data changes externally
That issue is harder to work around.. For example, if only the data is provided, the JSON Schema is computed from it, and then the UI model is computed from the schema.
When the data is updated externally, I would therefore expect the computed schema and UI model to be updated as well, since the underlying data has changed. However, this recomputation does not happen when the data is updated from the outside, which leads to inconsistencies.

there is an issue with the schema generator that does not produce the expected result in all cases and hence it was also changed. - again can't fix that as a workaround.

Yes these two use cases make sense to me, NOT storing schema and UI Schema in the component (or storing and comparing to not always update the instance with no actual content changes) but regenerating them when new data is retrieved makes sense to me on a high level without looking into the details yet.

So what we want is to introduce another event besides 'change' to support v-model:data ?

sdirix

Yes, I think we need to cleanly differentiate between "controlled" data and "uncontrolled" data. This becomes even more clear now with the schema regeneration.

Alternative for the schema generation: We could also think about NOT handling the schema regeneration within JSON Forms and expecting the caller of JSON Forms to remount us, e.g. via a key, so we don't need to handle that case within JSON Forms. However I would be fine to keep it within JSON Forms but only for the "controlled" mode, so that we don't regenerate on simple user edits.

kchobantonov · 2026-05-06T21:55:08Z

Yes, I think we need to cleanly differentiate between "controlled" data and "uncontrolled" data. This becomes even more clear now with the schema regeneration.

Alternative for the schema generation: We could also think about NOT handling the schema regeneration within JSON Forms and expecting the caller of JSON Forms to remount us, e.g. via a key, so we don't need to handle that case within JSON Forms. However I would be fine to keep it within JSON Forms but only for the "controlled" mode, so that we don't regenerate on simple user edits.

There are now checks for schema-less usage: when the data prop changes, we compare the newly generated schema with the current one and only update the schema/UI schema if the data structure actually changed.

kchobantonov · 2026-07-18T04:54:04Z

@sdirix Do we still plan to merge this PR, or have we decided not to pursue this approach?

I'm asking because, besides the additionalErrors issue with form data, this PR also includes a fix for schema generation. Instead of:

return gen.schemaObject(instance);

it uses:

return gen.property(instance);

which is more correct, as it allows the generator to properly detect whether the input is a simple type, an array, or an object and generate the appropriate schema.

If we're no longer considering merging this PR, I can create a separate PR containing just the schema generation fix and then close this one.

sdirix · 2026-07-21T09:46:01Z

I will re-review this PR today. Let's see.

sdirix · 2026-07-21T11:03:54Z

@sdirix Do we still plan to merge this PR, or have we decided not to pursue this approach?

I'm asking because, besides the additionalErrors issue with form data, this PR also includes a fix for schema generation. Instead of:

return gen.schemaObject(instance);

it uses:

return gen.property(instance);

which is more correct, as it allows the generator to properly detect whether the input is a simple type, an array, or an object and generate the appropriate schema.

If we're no longer considering merging this PR, I can create a separate PR containing just the schema generation fix and then close this one.

@kchobantonov I re-reviewed everything today, including testing the behavior in the Vuetify dev app. Most of it is in good shape and I would recommend to merge it.

The generateJsonSchema fix is correct and now well covered by tests.
The generated-UI-schema fallback works. We would just like it moved into generateDefaultUISchema itself so React and Angular benefit as well.

The one part we don't want to merge as is: this.dataToUse = newEvent.data. It silently changes the semantics of the data prop for all Vue users and makes Vue diverge from React and Angular. As I wrote before we need to cleanly differentiate between "controlled" and "uncontrolled" data.

Concretely that means: keep the data prop semantics as they are and add the update:data event from the previous discussion, so v-model:data works out of the box. The additionalErrors use case is then solved by binding v-model:data, without JSON Forms mixing controlled and uncontrolled state internally. I left inline comments with the details.

If you'd rather land the schema generation fix quickly, splitting it into its own PR is fine with us and should merge fast. But if you're up for the update:data change, we can just as well finish everything here. Thanks again for driving this!

kchobantonov · 2026-07-30T21:40:59Z

@sdirix all reported issues are now addressed - please review.

EclipseSourceAI

Note

Autonomous AI review.

This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. Noting why helps, since replies are read and taken into account in follow-up reviews.

Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.

To get an updated review after pushing changes, a maintainer may re-request a review from this account.

Running in Eclipse Enclave, submitted via review-guard-mcp

Fixes the additionalErrors resetting form data in the Vue JsonForms component by adding an update:data event for v-model:data support, plus schema/uischema regeneration when the data prop changes externally in schema-less mode. Also carries two @jsonforms/core generator fixes: generateJsonSchema now handles non-object roots (primitives, arrays, undefined, null) via gen.property, and generateDefaultUISchema no longer returns null.

All previous review points from sdirix and lucas-koehler look addressed: the dataToUse sync is gone, the uischema fallback moved into the generator so React and Angular benefit, echo detection compares against jsonforms.core.data, and the generator tests cover the new cases.

Verified locally: build and lint clean, all package tests pass (the angular-material failure in my first run was a missing CHROME_BIN, not the PR). I also drove the Vuetify dev app: the additional-errors example keeps edited data across repeated "Add additional error" clicks, #dynamic regenerates from Name to Id on external data change, and root-level primitive data (42, "hello") renders a single working control with no console errors.

Where to focus: the new data watcher in packages/vue/src/components/JsonForms.vue, specifically the !this.schema guard (breaks the legal schema: false case, verified), and whether the two public core generator behavior changes should get MIGRATION.md entries under the existing 3.9 section. The rest of the inline comments are smaller cleanups.

EclipseSourceAI · 2026-08-03T21:40:09Z

+      const isSameAsCurrentData = newData === this.jsonforms.core.data;
      this.dataToUse = newData;
+
+      if (!this.schema && !isSameAsCurrentData) {


!this.schema breaks schema: false, which is a legal JSON Schema and allowed by the prop type. Init uses ?? (master), so schemaToUse starts out as false and then gets silently replaced by a generated object schema on the first data prop change. Use this.schema === undefined to stay consistent.

EclipseSourceAI · 2026-08-03T21:40:09Z

-
 const EMPTY: ErrorObject[] = reactive([]);

+const getSchemaGeneratorInput = (data: any) => (data === undefined ? {} : data);


With generateJsonSchema(undefined) now returning {}, this local undefined -> {} mapping is dead weight and keeps Vue diverging from React, which passes data straight through (master). Same argument that moved the uischema fallback into the generator applies here: drop the helper and pass dataToUse.

EclipseSourceAI · 2026-08-03T21:40:10Z

      );
    },
    eventToEmit(newEvent) {
+      this.$emit('update:data', newEvent.data);


mounted() emits only change, so the initial core state never reaches a v-model:data parent. A middleware that transforms data inside Actions.init is then lost on the next unrelated prop change (additionalErrors hands dataToUse back into updateCore), which is the same bug class this PR fixes. Emit update:data in mounted() too.

EclipseSourceAI · 2026-08-03T21:40:11Z

    generateUISchema(jsonSchema, [], prefix, '', layoutType, rootSchema),
    layoutType
-  );
+  ) ?? createLayout(layoutType);


generateDefaultUISchema returning an empty layout instead of null is a public API behavior change and deserves a MIGRATION.md entry under the existing 3.9 section (master). It also makes the foundUISchema !== null branches unreachable in both CombinatorProperties implementations (react, vue), which could be cleaned up here.

EclipseSourceAI · 2026-08-03T21:40:12Z

  const gen = new Gen(findOption);

-  return gen.schemaObject(instance);
+  return gen.property(instance);


Good fix, but it changes results for existing callers: generateJsonSchema([1, 2]) used to return an object schema with "0"/"1" properties and now returns {type: 'array', items: {type: 'integer'}}. Worth a MIGRATION.md entry alongside the uischema change.

EclipseSourceAI · 2026-08-03T21:40:12Z

  methods: {
    onChange(event: JsonFormsChangeEvent) {
-      this.data = event.data;
+      console.log(event);


console.log(event) doesn't show a reader anything. Either drop the handler from the sample or use it for something real (e.g. storing event.errors). The data prop docs above also still don't say it is controlled and must be kept in sync, which is what makes v-model:data the recommended binding.

preserve the form data so when we update the additionalErrors the dat…

e41c15d

…a is not reset to the initial state

Merge branch 'master' into preserve-edit-data

a6a21e3

kchobantonov added 2 commits August 23, 2025 18:09

also update the schema and uischema when they depend on the data and …

d5218b1

…the data is changed

properly generate schema in case when the data is not an object but s…

0b979c0

…imple type like number, string, array and etc.

kchobantonov and others added 4 commits August 23, 2025 23:43

always return valid UISchemaElement, in case of empty schema this was…

ebe262b

… returning null

revert back the ability to return null from the generate but default …

51aa0bc

…to non null uischema in JsonForms

only for undefined use emtpy data for others the core can handle those

b27ce7f

Merge branch 'master' into preserve-edit-data

87bfaa6

lucas-koehler force-pushed the master branch 4 times, most recently from 24c8cb2 to d86047e Compare October 24, 2025 10:26

kchobantonov added 2 commits November 7, 2025 08:29

Merge branch 'master' into preserve-edit-data

e413248

Merge branch 'master' into preserve-edit-data

7bde214

Merge branch 'master' into preserve-edit-data

6f736d2

lucas-koehler requested changes Nov 19, 2025

View reviewed changes

Comment thread packages/vue/src/components/JsonForms.vue Outdated

Comment thread packages/vue/src/components/JsonForms.vue Outdated

Comment thread packages/vue-vuetify/dev/views/ExampleView.vue

Merge branch 'master' into preserve-edit-data

4803dec

lucas-koehler requested a review from sdirix December 12, 2025 11:19

kchobantonov requested a review from lucas-koehler December 19, 2025 13:42

kchobantonov added 3 commits January 23, 2026 11:55

Merge branch 'master' into preserve-edit-data

fd04fe0

Merge branch 'master' into preserve-edit-data

d9de6ab

Merge branch 'master' into preserve-edit-data

63f16bf

sdirix reviewed Mar 5, 2026

View reviewed changes

Comment thread packages/core/src/generators/schema.ts

Comment thread packages/vue/src/components/JsonForms.vue Outdated

kchobantonov and others added 5 commits March 27, 2026 10:26

Merge branch 'master' into preserve-edit-data

972c194

Merge branch 'master' into preserve-edit-data

8353196

add test case for schema generation for primitive types

aa5b3c4

preserve undefined Vue form data and type primitive schema generation

4af7884

avoid regenerating Vue schemas when parent echoes unchanged form shape

66e5c00

kchobantonov requested a review from sdirix April 19, 2026 02:03

kchobantonov and others added 2 commits May 7, 2026 00:22

Merge branch 'master' into preserve-edit-data

7194ebf

organize imports and code format

75a7203

kchobantonov added 4 commits June 7, 2026 01:11

Merge branch 'master' into preserve-edit-data

1299f70

Merge branch 'master' into preserve-edit-data

82e985c

Merge branch 'master' into preserve-edit-data

c925284

Merge branch 'master' into preserve-edit-data

f02d205

sdirix requested changes Jul 21, 2026

View reviewed changes

kchobantonov and others added 2 commits July 30, 2026 09:49

Merge branch 'master' into preserve-edit-data

c1a54b7

add fixes based on the code review

18d6212

kchobantonov requested a review from sdirix July 30, 2026 21:40

Merge branch 'master' into preserve-edit-data

ade1fa2

sdirix requested a review from EclipseSourceAI August 3, 2026 21:22

EclipseSourceAI reviewed Aug 3, 2026

View reviewed changes


		const EMPTY: ErrorObject[] = reactive([]);

		const getSchemaGeneratorInput = (data: any) => (data === undefined ? {} : data);

Uh oh!

Conversation

kchobantonov commented Aug 12, 2025

Uh oh!

netlify Bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for jsonforms-examples ready!

Uh oh!

kchobantonov commented Aug 12, 2025

Uh oh!

coveralls commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdirix commented Aug 13, 2025

Uh oh!

kchobantonov commented Aug 15, 2025

Uh oh!

kchobantonov commented Aug 24, 2025

Uh oh!

kchobantonov commented Sep 27, 2025

Uh oh!

kchobantonov commented Nov 10, 2025

Uh oh!

lucas-koehler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kchobantonov commented Dec 7, 2025

Uh oh!

kchobantonov commented Dec 7, 2025

Uh oh!

sdirix commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lucas-koehler commented Dec 12, 2025

Uh oh!

kchobantonov commented Dec 24, 2025

Uh oh!

sdirix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kchobantonov commented May 6, 2026

Uh oh!

kchobantonov commented Jul 18, 2026

Uh oh!

sdirix commented Jul 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sdirix commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kchobantonov commented Jul 30, 2026

Uh oh!

EclipseSourceAI left a comment

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

EclipseSourceAI Aug 3, 2026

netlify Bot commented Aug 12, 2025 •

edited

Loading

coveralls commented Aug 13, 2025 •

edited

Loading

sdirix commented Dec 12, 2025 •

edited

Loading

sdirix commented Jul 21, 2026 •

edited

Loading