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

feat(conform-react): stop relying on the state snapshot #467

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

edmundhung
Copy link
Owner

@edmundhung edmundhung commented Feb 20, 2024

Background

Conform uses useSyncExternalStore with subjects tracked by a proxy to achieve fine-grained subscription. This works fine generally.. but not in a callback the way you might want! As an example, imagine you are trying to log the message out if the input is not empty:

function Exmaple() {
  const [form, fields] = useForm({
    // ...
  }) 

  return (
    <form>
        <input
            {...getInputProps(fields.message)}
            onChange={event => {
                // Trying to access the latest value of message:
                if (fields.message.value) {
                    console.log(fields.message.value)
                }
            }}
        />
    </form>
  );
}

It will never logged anything unfortunately. Because Conform has no idea whether you need the value state and never subscribe it. The fields.message.value is outdated with the undefined value. To solve this, a kinda awkward approach is to explicitly subscribe it during render:

function Exmaple() {
  const [form, fields] = useForm({
    // ...
  })
  // Now the value is subscribed and Conform will trigger a re-render everytime value is changed.
  const messageValue = fields.message.value;

  return (
    <form>
        <input
            {...getInputProps(fields.message)}
            onChange={event => {
                if (messageValue) {
                    console.log(fields.message.value)
                }
            }}
        />
    </form>
  );
}

FYI, react-hook-form suffers from similar problem with proxy based subscription and you will find similar suggestions on their repository.

The reason why this happened is because useSyncExternalStore takes a snapshot from the form context and will use the same snapshot until a re-render is triggered. The form context is aware of the value changed internally but just the metadata is populating the outdated data from the snapshot. However, this is not a bug on React but rather an intentional design to avoid potential tearing with suspense.

Solutions

I have considered 2 solutions:

  1. A new API to return the latest metadata
function Exmaple() {
  const [form, fields] = useForm({
    // ...
  }) 

  return (
    <form>
        <input
            {...getInputProps(fields.message)}
            onChange={event => {
                // `fields.message.latest` returns a field metadata using the latest metadata
                // Similar, you can access the latest dirty state by calling `form.latest.dirty`
                if (fields.message.latest.value) {
                    console.log(fields.message.latest.value)
                }
            }}
        />
    </form>
  );
}
  1. Make all metadata always return the latest metadata
function Exmaple() {
  const [form, fields] = useForm({
    // ...
  }) 

  return (
    <form>
        <input
            {...getInputProps(fields.message)}
            onChange={event => {
                // Nothing changed. This will work just fine
                // The value is populated from the form context instead of the snapshot 
                if (fields.message.value) {
                    console.log(fields.message.value)
                }
            }}
        />
    </form>
  );
}

Personally, I prefer solution 2 (which is what implemented in this PR) as I believe the impact of tearing is minimal in the context of a form in which the whole form will always suspense together instead of some pieces suspense while some not. It will take sometime until the react ecosystem adopts the new paradigm anyway and Conform might already reach version 2 by that time with a different design. But I would love to hear what the community think.

@edmundhung edmundhung added the feedbacks welcome Looking for feedbacks label Feb 20, 2024
@coji
Copy link
Contributor

coji commented Feb 21, 2024

I was just getting into this issue and thanks to your help I was able to workaround it. I'm glad it's simple and easy to understand 2. I also found the difference between initialValue and defaultValue to be complicated, so having two similar things seems confusing.

@edmundhung edmundhung force-pushed the stop-relying-on-state-snapshot branch from f9092fa to 40bfb5a Compare March 27, 2024 13:30
Copy link

cloudflare-workers-and-pages bot commented Mar 27, 2024

Deploying conform with  Cloudflare Pages  Cloudflare Pages

Latest commit: 620e574
Status: ✅  Deploy successful!
Preview URL: https://40bcbae8.conform.pages.dev
Branch Preview URL: https://stop-relying-on-state-snapsh.conform.pages.dev

View logs

@edmundhung edmundhung force-pushed the stop-relying-on-state-snapshot branch 2 times, most recently from 7ab2164 to 114c258 Compare March 28, 2024 17:06
@edmundhung edmundhung force-pushed the stop-relying-on-state-snapshot branch from bed3598 to 620e574 Compare April 1, 2024 15:01
@edmundhung edmundhung merged commit ddbbcc4 into main Apr 1, 2024
3 checks passed
@edmundhung edmundhung deleted the stop-relying-on-state-snapshot branch April 1, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedbacks welcome Looking for feedbacks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants