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 v6]: navigate() and <Navigate> doesn't produce the same result #7962

Closed
eakl opened this issue Aug 19, 2021 · 7 comments
Closed

[Bug v6]: navigate() and <Navigate> doesn't produce the same result #7962

eakl opened this issue Aug 19, 2021 · 7 comments

Comments

@eakl
Copy link

eakl commented Aug 19, 2021

What version of React Router are you using?

17.0.1

Steps to Reproduce

CSB: https://codesandbox.io/s/react-router-useparams-lcn4d?file=/src/App.js

This CSB reproduces what I have in local but - I don't know why - useParams() doesn't capture the parameter.

The whole code to reproduce the bug is short though

You need to comment/uncomment the following lines

navigate(`..`, { replace: true });
// navigate(`/main`, { replace: true });
// return <Navigate to={`..`} replace={true} />;
// return <Navigate to={`/main`} replace={true} />;

Expected Behavior / Actual behavior

Path: /main/12345
Action: Both navigate('..', { replace: true }) and navigate('/main', { replace: true })
Expected behavior: redirect to /main
Actual behavior: stays at /main/12345

If you replace navigate(...) by:
return <Navigate to={'..'} replace={true} /> or
return <Navigate to={'/models'} replace={true} />

it redirects to /main as expected.

@chaance
Copy link
Collaborator

chaance commented Aug 19, 2021

Because you are rendering the <Home /> component in your top-level home route, it needs an Outlet to render the nested routes. The reason useParams doesn't give you an id param in this case is because it's being rendered by the /home route, not the /home/:id route.

If you include the Outlet you'll see <Home /> is rendered again by the correct nested route with the id. https://codesandbox.io/s/react-router-useparams-forked-47t7b?file=/src/App.js

Now, that being said, we do intend to support this feature. @mjackson describes this in #7960

Regarding the issue with Navigate, what you described isn't showing up in your sandbox. Do you have another link I can take a look at to illustrate that more clearly?

@eakl
Copy link
Author

eakl commented Aug 20, 2021

Thanks @chaance for your reply.

I am confuse now.

I have seen here #7285 that the recommended way to do optional parameters is by nesting routes with the same element so that /home and /home/:id renders the same component <Home />. Because the component is rendered at the top-level route I should use <Outlet />. But then I need to include <Outlet /> in the component itself and then render it twice.

The use case here is fairly simple:

  1. Both /home and /home/:id renders <Home />
  2. Inside <Home /> checks if :id exists.
  3. If yes, fetch data relative to :id. If not, redirects to /home

This is at (3.) where navigate(...) and <Navigate ... /> where behaving differently when landing directly to /home/:id

Also, is there a way to console.log() the matched route inside a component so I can debug it better?

@eakl
Copy link
Author

eakl commented Aug 21, 2021

Here you go:

https://codesandbox.io/s/react-router-useparams-lcn4d?file=/src/App.js

I have refactored the CSB to get closer to what I want to achieve

Change in Route matching

<Route path="home">
  <Route path="/" element={<Home />} />
  <Route path=":actionOrId" element={<Home />} />
  <Route path=":actionOrId/:action" element={<Home />} />
</Route>

I have removed element={<Home />} from the top level <Route path="home"> so that I render the Home component at sub-level and I don't have to use <Outlet /> but still get params inside Home. Is that the correct way to solve the problem above?

Optional parameters

As v6 stopped supporting optional parameter, I am doing validation inside the Home component with a parameter that can take different value format.

The possible cases are:

  • /home => Show all resources
  • /home/1234 => Show resource '1234'
  • /home/create => Create new resource
  • /home/1234/edit => Edit resource 1234
  • /home/123456789 => every other pathname are invalide and should redirect to /home

Line 45 to 48 of the CSB is the validation

navigate(...) vs. <Navigate ... />

If you comment/uncomment lines 46 and 47 you should normally have different behavior. <Navigate ... /> works but navigate(...) doesn't

@timdorr
Copy link
Member

timdorr commented Aug 31, 2021

You need to run navigate inside of a useEffect. There's even a warning about this in your example's console. If you wrap it, it starts working just fine.

@timdorr timdorr closed this as completed Aug 31, 2021
@eakl
Copy link
Author

eakl commented Aug 31, 2021

Thanks @timdorr. Why does navigate(...) can't work outside useEffect, in an event handler in response to a user action? Any reason?

@timdorr
Copy link
Member

timdorr commented Aug 31, 2021

It can work inside of an event handler just fine, but not directly inside of the render function of the component. That creates a side effect on every render, which you should always put inside of useEffect.

@eakl
Copy link
Author

eakl commented Aug 31, 2021

Got it. Thanks @timdorr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants