facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.09k stars 46.65k forks source link

React 18: react-router@v5 is breaking in the Strict Mode (strict effects) #21674

Closed Jack-Works closed 2 years ago

Jack-Works commented 3 years ago

https://github.com/ReactTraining/react-router/issues/7870

I do not have permission to post https://github.com/reactwg/react-18/discussions. Please open and pin a new issue in that repo to list all widely-used library that does not work with React 18 or need special handling.

gaearon commented 3 years ago

Hi! Can you clarify what would be the purpose of an issue like this? We’re not yet asking application authors to try the release.

The Alpha is focused on the library maintainers, and there will be a several months period before the Beta. It’s expected things would need adjustment early on, and I’m worried making a list of popular libraries before the authors even got a chance to try the release might be a bit premature.

We are committed to working with library authors and helping find recommendations where they’re unclear, including on this issue tracker. I’m just not sure that making a public list is the right way to kick this off.

What do you think?

Jack-Works commented 3 years ago

We’re not yet asking application authors to try the release.

People won't listen (like me).

For example, TikTok has upgraded to React 18 already (https://twitter.com/Brooooook_lyn/status/1402632529270632456?s=20)

Since people want to try out fresh things immediately, I think a list of what is blocking people from upgrading is helpful and can reduce other one's time-wasting.

eps1lon commented 3 years ago

For example, TikTok has upgraded to React 18 already (twitter.com/Brooooook_lyn/status/1402632529270632456?s=20)

This looks like they're just testing integration, not pushing React 18 to production.

I personally don't see any issue with app devs trying the alpha already. From how I understood it, this isn't generally recommended because most apps use at least one library. So it might be frustrating if you want to try it out, but upstream isn't ready. So right now the goal is to work the problem bottom up so that library maintainers aren't flooded with issues regarding "React 18 compat when?". But that's just from my perspective.

Rather than listing libraries that aren't ready (putting negative pressure on those maintainers) we may want to start a list of the libraries that are compatible. Though right now is probably too early.

can reduce other one's time-wasting.

I think there's a mismatch of expectations here. Upgrading to React 18 right now is most likely not the most productive task. Upgrading and facing issues right now should be the expectation so considering it a "waste of time" isn't very constructive. The React working group is actively looking for such issues right now. Identifying these issues is the exact opposite of "wasting time".

gaearon commented 3 years ago

I personally don't see any issue with app devs trying the alpha already.

Yup — the issue isn't with trying, but with expectations. It is expected things might be broken (especially in Strict Mode, since it's gotten stricter), so we don't want people to create negative pressure on maintainers.

I think there's a mismatch of expectations here. Upgrading to React 18 right now is most likely not the most productive task. Upgrading and facing issues right now should be the expectation

Exactly.

Jack-Works commented 3 years ago

Sorry for my word choice. I'm not mean to use the negative meaning word. What I mean is, if any app dev wants to try out, and they found the library they're using is currently not working, they can give up the upgrading so they'll not go through & resolve the problem that previous people did.

Yes, I agree this will give pressure on the library authors...

iamfotx commented 3 years ago

It took me 3 hours today to figure out why React Router wasn't updating the view with React@experimental though Link was updating the URL. And in the end, it was the Strict Mode. So, no complaints. I was expecting it this way.

popuguytheparrot commented 2 years ago

Here with React 18 RC 0 reproduce https://codesandbox.io/s/react-router-sidebar-forked-mzyvs?file=/index.js

eps1lon commented 2 years ago

@popuguytheparrot Considering react-router v6 was released recently, could somebody check if the issue also reproduces in that version? It's likely that v5 will not receive any more updates so targeting v6 is more likely to result in a fix.

Jack-Works commented 2 years ago

v6 is good.

popuguytheparrot commented 2 years ago

@eps1lon https://codesandbox.io/s/react-router-sidebar-forked-bc21y?file=/example.js Here v6 and React 18 RC 0. Seems OK.

eps1lon commented 2 years ago

v6 is good.

Thanks for confirming. Closing since this is fixed in their latest release. For support of older versions please file an issue in their repository.

popuguytheparrot commented 2 years ago

Note: If your application uses history outside of react, then updating to version 6 will be difficult

jiayihu commented 1 year ago

Note: for whoever wants to stick with v5, wrapping the BrowserRouter before StrictMode works:

<BrowserRouter>
  <StrictMode></StrictMode>
</BrowserRouter>

I guess it avoids the double mounting/unmounting introduced in React 18.

hichemfantar commented 1 year ago

Note: for whoever wants to stick with v5, wrapping the BrowserRouter before StrictMode works:

<BrowserRouter>
  <StrictMode></StrictMode>
</BrowserRouter>

I guess it avoids the double mounting/unmounting introduced in React 18.

The latest v5 update should have fixed the strict mode problem.

Naitik-ReactJs commented 1 year ago

It took me 3 hours today to figure out why React Router wasn't updating the view with React@experimental though Link was updating the URL. And in the end, it was the Strict Mode.