edmundhung / conform

A type-safe form validation library utilizing web fundamentals to progressively enhance HTML Forms with full support for server frameworks like Remix and Next.js.
https://conform.guide
MIT License
1.83k stars 101 forks source link

The form value is out of sync if we are dispatching multiple intents in a row #400

Open edmundhung opened 7 months ago

edmundhung commented 7 months ago

Describe the bug and the expected behavior

Originally reported by @Pouet-- on https://github.com/edmundhung/conform/discussions/331#discussioncomment-8307261

Conform version

v1.0.0-rc.1

Steps to Reproduce the Bug or Issue

I reproduced the "bug" in a codesandbox, you can try to drag the first level items and drop them into the sub-level node : the number of children updates accordingly but the list is not correctly displayed. However, if you add an item in the sub-list after that, it retrieves the lost values.

What browsers are you seeing the problem on?

No response

Screenshots or Videos

No response

Additional context

No response

edmundhung commented 7 months ago

@Pouet-- Let's move the discussion here. The issue you see will be resolved if you identify the child type with initialValue instead of value on these 2 lines:

childFieldset.type.initialValue === TreeNodeType.LOGIC
childFieldset.type.initialValue === TreeNodeType.FILTER

The issue is that Conform's value get out of sync and mess up if you dispatch more than one intents in one callback. Definitely something I need to look into further. 😅

Thanks again for preparing the repo!

Pouet-- commented 7 months ago

I changed parts of my code since v1 is released to use value on my fields, did not expect the problem to be related to this. Thank you very much for taking the time to look into this problem !

Pouet-- commented 6 months ago

For information, I hacked this problem with the following :

flushSync(() => {
  form.insert({ name: ..., defaultValue: ... });
});
flushSync(() => {
  form.remove({ name: ..., index: ... });
});

It waits for a re-render before launching the second operation, so the values are correctly filled, but this triggers 2 re-renders

edmundhung commented 6 months ago

In v1.0.3, Conform has all form intents wrapped in a flush sync already so there is no need to wrap it yourself.

QzCurious commented 4 months ago

Hi, @edmundhung. I'd argue conform wrap form intent (maybe I should say wrap programatic for intent) is not a good decision. Maybe state flushSync() way on doc would be better.

One significant drawback wrapping every form intent with flushSync() I had is that I hook up field values to render / not-render something. If I'd want to show three things on screen, it would show one by one instead of three at the same time.