Nested Values Disappear when using StrictMode and `pushValue`

dlindahl commented 5 months ago

Describe the bug

@dlindahl I believe what you're describing is a separate issue that pertains to pushValue. Can you make a new issue?

Originally posted by @crutchcorn in https://github.com/TanStack/form/issues/685#issuecomment-2089490042

I am seeing an issue with React's StrictMode and pushValue with an array field where it doesn't appear to do anything. Under the hood, I believe it is adding the value as expected but then immediately reverting the field value to whatever the previous value was.

Your minimal, reproducible example

https://stackblitz.com/edit/tanstack-form-rxy9kq?file=src%2Findex.tsx&preset=node

Steps to reproduce

I took the array example from the docs and changed it in the following ways:

Wrap App in StrictMode.
Change [Array<{ name: string; age: number }>](people: [] as Array<{ name: string; age: number }>) to people: [] as Array<String>
Clicking on the "Add person" button does not result in a new row.

Expected behavior

As a user, I expect the "Add person" button to push a new value to the field's internal array state.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Chrome 123.0.6312.87 (Official Build) (arm64)

TanStack Form adapter

None

TanStack Form version

v0.19.4

TypeScript version

No response

Additional context

Also of note is when you leave the people value to be an array of objects, StrictMode works fine. Its only when the underlying value is an array of strings that it breaks. If I had to guess, I'd say there is a value check somewhere that is failing since an empty string is "falsey", but I have no idea what the root cause would be.

JortRoelofs commented 5 months ago

I'm also experiencing this issue. It's not the value check as you guessed, as calling pushValue with a non-empty string fails as well. The problem extends to swapValues and removeValue too where they exhibit weird behaviour that I think can be attributed to unsafe state, as StrictMode obviously runs twice to highlight such problems.

Balastrong commented 5 months ago

I'm not sure why this is happening, but looks like the Field components get unmounted and in the callback here we delete the field.

https://github.com/TanStack/form/blob/3e827afac920ba312691670a93724edc9fcfed2b/packages/form-core/src/FieldApi.ts#L403-L409

In fact, if you add preserveValue in <form.Field key={i} name={people[${i}]} preserveValue> this behavior is skipped and the values are getting pushed as expected.

This seems to be the same root cause as #699

crutchcorn commented 4 months ago

@Balastrong I think you're on the right track here, but the problem is that deleteField and creating a field should create an equilibrium in effects if ran back-to-back, ala StrictMode.

If you're able to get to this before I'm able to investigate, let's see if we can iron out why creating a field with numbers and nested values disappears. Look at the mounting behavior, not the unmounting behavior.

crutchcorn commented 4 months ago

The problem that's occuring here is incredibly specific to both:

Our unique framework agnostic tech stack
The way StrictMode works with dual-rendering
The way the VDOM works with React

TLDR what's happening is that:

We are mounting the people[0] FieldAPI, which adds it to the FormAPI.state.values array
We then call the mount unsubscribe on the people[0] FieldAPI, which removes it from FormAPI.state.values array
This in turn notifies the people FieldAPI, which triggers the change on the array itself and re-renders the VDOM list
This VDOM list is updated and therefore unmounts people[0], which runs the cleanup on the FieldAPI on people[0], which is why it never renders

To solve this, I think we need to introduce a notification priority system with a bottom-up notification tree (notify people[0], then people, then '').

crutchcorn commented 4 months ago

Disregard, the challenge is that defaultValue is undefined, which never triggers this line of code:

https://github.com/TanStack/form/blob/main/packages/form-core/src/FieldApi.ts#L448-L450

As a result, the code goes through the following processes:

1a) First (dummy StrictMode) render: i) Sets up the FieldAPI for people and: ii) people[0] 1b) First (dummy StrictMode) cleanup: iii) Deletes people[0] iiii) Deletes people, removing the array from FormAPI.store.state.values 2a) Second (real) render: v) Creates people[0] first, but does not trigger the inform the form creation vi) Then handles people, which recreates the (now empty) array

Because the people array is now empty, it creates a:

3a) Second (real) cleanup

To solve this temporarily, simply add:

defaultValue={""}

To here:

<form.Field key={i} defaultValue={""} name={`people[${i}]`}/>

PR with fix soon

dlindahl commented 4 months ago

@crutchcorn While this does solve the disappearing value issue in the reproduction case, I did notice that the example now prints a console warning from React when pushing a new item:

react-dom_client.js?v=7456daa3:519 Warning: Cannot update a component (`Field`) while rendering a different component (`Field`). To locate the bad setState() call inside `Field`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render
    at Field (https://tanstackformcyvv78-izrn--3001--dc4d7514.local-credentialless.webcontainer.io/node_modules/.vite/deps/@tanstack_react-form.js?v=36f87496:2410:3)
    at APIField
    at div
    at Field (https://tanstackformcyvv78-izrn--3001--dc4d7514.local-credentialless.webcontainer.io/node_modules/.vite/deps/@tanstack_react-form.js?v=36f87496:2410:3)
    at APIField
    at form
    at div
    at App (https://tanstackformcyvv78-izrn--3001--dc4d7514.local-credentialless.webcontainer.io/src/index.tsx:6:16)

I am currently trying to track down a bug in my own code where newly pushed values are (still) being dropped in certain rendering contexts and am seeing the same warning. It is unclear to me if this warning is part of the problem or not, but wanted to rule it out if possible.

I've updated the example to use the latest release: https://stackblitz.com/edit/tanstack-form-cyvv78?file=src%2Findex.tsx

I feel like this example is pretty straight forward so I do not understand what React is complaining about unless it has something to do with the internals of @tanstack/react-form. Any thoughts?

chidiwilliams commented 4 months ago

I've noticed the fix still doesn't work for nested objects within the array in StrictMode.

Taking the original example from the docs and wrapping with Strict Mode, adding a new Person object three times leads to: [{"age":0},{"age":0},{"name":"","age":0}].

https://stackblitz.com/edit/tanstack-form-ien81g?file=src%2Findex.tsx

crutchcorn commented 4 months ago

@dlindahl that warning comes from TanStack React Form and IDK why. It can be safely ignored tho

crutchcorn commented 4 months ago

@chidiwilliams let's make a new issue. I think this is a different underlying problem, even tho it looks very similar

dlindahl commented 4 months ago

@crutchcorn I'll do some more debugging on my end but I'm not 100% sure it can be ignored. In my very specific implementation of field rendering on top of @tanstack/react-form, I am seeing the state of the form adding a new value on click and then immediately removing that item again only under StrictMode. I can reduce the problem enough that it occurs in once scenario but not another but its been hard to suss out exactly where the problematic code is coming from.

I'll try to circle back around if I ever find that cause. Thanks for looking into this though!

crutchcorn commented 4 months ago

@dlindahl what version are you using? The most recent patches issues with this.

TanStack / form