forge42dev / remix-hook-form

Open source wrapper for react-hook-form aimed at Remix.run
MIT License
330 stars 27 forks source link

`formState.isSubmitting` is `true` when the loaders are refetching #56

Closed MonsterDeveloper closed 2 months ago

MonsterDeveloper commented 9 months ago

According to this line of the hook, the isSubmitting state is true when the navigation state (or a fetcher) is not "idle".

This includes a moment after the response from action is received but the loaders are revalidating.

I believe the check should rather be === "submitting", as per Remix docs:

submitting - A route action is being called due to a form submission using POST, PUT, PATCH, or DELETE

This would make sure that formState.isSubmitting is actually reflecting the submitting state, and not loading.

What do you think?

AlemTuzlak commented 9 months ago

@MonsterDeveloper the issue here is if you submit to an action and that action redirects you'd have a moment where the form is "not submitting" but it's not idle either where you should be able to tinker with fields

MonsterDeveloper commented 9 months ago

Got it. The reason I'm asking is, when for instance the loaders take a while to refetch, there's a moment when the response from the action is received (an so the error state is shown in the UI), but the isSubmitting is still true. So the user, on the one hand, has a response from the server that something is not OK, yet the form is still indicating that it's submitting.

Here's a small sketch: image

The moment I'm talking about is in yellow.

AlemTuzlak commented 9 months ago

Hmm yes that's a valid point, it's hard to decide on what would be the expected experience here, should you expect it to be only while submitting or while redirecting too as the default 🤔

MonsterDeveloper commented 9 months ago

True. On the other hand, if isSubmitting only reflects the submitting navigation state, there will be a moment between the response from the action and redirect, where a form is not in a loading state, causing a flicker (yellow rect):

image

Is there a way of detecting the type of response returned from an action (i.e. json or redirect)? If so, we could make isSubmitting true when the navigation state is equals to submitting if the action returned a non-redirect response, and keep the current behavior (i.e. !== "idle") if that's a redirect.

romualdr commented 8 months ago

My 2cts; I believe your 2nd example @MonsterDeveloper reflects how most "cache revalidated" libs works (like swr) - it doesn't display that it's "submitting" anymore after the initial server response, though it can flash in case the data was updated at revalidation.

I believe that "isSubmitting" should behave like your 2nd example, and maybe have a "isRevalidating" flag to handle the yellow part ? This way, any dev could implement the wanted behaviour according to their needs: Either listen to isSubmitting, isRevalidating or both (isSubmitting || isRevalidating) ?

MonsterDeveloper commented 8 months ago

I wouldn't go far from the original react-hook-form API: CleanShot 2024-01-11 at 21 37 45

But it's up to @AlemTuzlak to decide. Now when I have used the library enough myself, I don't think it's a big deal tbh.

huijiewei commented 5 months ago

When using Link to navigate to other pages, isSubmitting is also true, which is not as expected.

AlemTuzlak commented 5 months ago

@huijiewei I think I've fixed that issue with 4.3.1

AlemTuzlak commented 2 months ago

@romualdr Correct but I'd rather go with the current flow in the "normal" case as most people would expect their forms to stay disabled until it's "done done", for those advanced scenarios you can in any case rely on remix pending states and show optimistic UI with fetchers, I'll close this issue and thank you to the @MonsterDeveloper for a detailed explanation and drawings