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

[Bug]: useFetcher options navigate true does not work #11627

Closed
Vardner opened this issue Jun 6, 2024 · 6 comments · Fixed by #11631
Closed

[Bug]: useFetcher options navigate true does not work #11627

Vardner opened this issue Jun 6, 2024 · 6 comments · Fixed by #11631
Labels

Comments

@Vardner
Copy link

Vardner commented Jun 6, 2024

What version of React Router are you using?

6.23.1

Steps to Reproduce

  1. Open demo
  2. At the front face you will see a table that indicates current navigation states for global and fetcher states
  3. Press "Trigger" button to fire submission
  4. See the resultsthe

Expected Behavior

  1. Global navigation state corresponds to the fetcher since "navigate: true" is specified
  2. Data revalidation happens only once for "PageB" component

Actual Behavior

  1. Global navigation state differs from the fetcher. When fetcher is "submitting", global state is "idle"
  2. When the page action finishes the job, the loader triggers for "PageB" component and both states become "loading"
  3. After the loader finishes the job, it starts all over again (you can see that global navigation state is still loading)
@Vardner Vardner added the bug label Jun 6, 2024
@Vardner
Copy link
Author

Vardner commented Jun 6, 2024

Additionally, I would like collaborators to explicitly mention that when "navigate" is set to false for fetcher, the global navigation state does not become "submitting". This should be outlined somewhere in the docs since it is an essential piece of information that cannot be found either in docs or StackOverflow / reddit / etc.

The terminology as "not cause navigation" is hard to understand and realize all its implications. Maybe you should describe what you mean by navigation in your lib somewhere in the docs and make link to that page so the newbies like me could understand it

@brophdawg11
Copy link
Contributor

Fetchers never trigger a navigation. navigate is only an option on <Form> and transitively useSubmit to opt-out of the underlying navigation. You cannot opt-into a navigation with a fetcher.

Looks like we might have a small TS bug that shows navigate as an option in fetcher.submit which we can fix up.

Additionally, I would like collaborators to explicitly mention that when "navigate" is set to false for fetcher, the global navigation state does not become "submitting"

Fetchers are never navigations - they are specifically when you don't want a navigation, as outlined in the top of the useFetcher docs.

The terminology as "not cause navigation" is hard to understand and realize all its implications

A navigation is defined as something that operates modifies the history stack (see https://reactrouter.com/en/main/start/concepts#history-and-locations). Link/Form/useNavigate/useSubmit all trigger navigations and under the hood call history.pushState or history.replaceState. navigate(-1) calls history.go(). Fetchers are the way top out out of that for in-page operations.

@brophdawg11 brophdawg11 linked a pull request Jun 7, 2024 that will close this issue
@Vardner
Copy link
Author

Vardner commented Jun 8, 2024

@brophdawg11 While you are not wrong about useFetcher it doesn't say explicitly what it means by "But sometimes you want to call a loader outside of navigation".
I find it confusing and maybe you should put this information in the FAQs.
You may do as you like, but you should take into account that you see no problem from a position of collaborator, meanwhile, a newbie read the docs and can't understand why it doesn't work. You are just losing a feedback btw

@brophdawg11 brophdawg11 added the awaiting release This issue have been fixed and will be released soon label Jun 13, 2024
@brophdawg11
Copy link
Contributor

We're always open to feedback on the documentation! I'm just not sure I understand what's lacking because, as you said, I am not looking at it through an end-user's eyes. Would you like to open a PR with the changes that would make it more clear?

The types have been updated in #11631 so that navigate is no longer shown via TS as a valid option on fetcher.submit

Copy link
Contributor

🤖 Hello there,

We just published version 6.24.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 6.24.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@brophdawg11 brophdawg11 removed the awaiting release This issue have been fixed and will be released soon label Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants