Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
edmundhung opened this issue Jan 30, 2024 · 5 comments · Fixed by #496
Open

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

edmundhung opened this issue Jan 30, 2024 · 5 comments · Fixed by #496
Assignees
Labels
bug Something isn't working

Comments

@edmundhung
Copy link
Owner

Describe the bug and the expected behavior

Originally reported by @Pouet-- on #331 (reply in thread)

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
Copy link
Owner Author

edmundhung commented Jan 30, 2024

@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--
Copy link

Pouet-- commented Feb 2, 2024

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 !

@edmundhung edmundhung self-assigned this Feb 2, 2024
@edmundhung edmundhung added the bug Something isn't working label Feb 2, 2024
@Pouet--
Copy link

Pouet-- commented Feb 21, 2024

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
Copy link
Owner Author

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

@edmundhung edmundhung reopened this Mar 11, 2024
@QzCurious
Copy link

QzCurious commented Apr 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants