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

React-router@3 with v7 beta #1210

Closed
amertak opened this issue Mar 22, 2019 · 7 comments
Closed

React-router@3 with v7 beta #1210

amertak opened this issue Mar 22, 2019 · 7 comments

Comments

@amertak
Copy link

amertak commented Mar 22, 2019

Do you want to request a feature or report a bug?

bug

What is the current behavior?

React router@3 not working.

Looks like that connect is not behaving the same way as in v6.
React-router@3 is testing React.isValidElement in ReactContext file and this check is returning false.

Maybe the reason is that the connected component is called later than before thus the check call resolves to false always?

How to reproduce:

https://codesandbox.io/s/2v0oy0xwrn

react-redux v6 - Error: Could not find "store" in the context of "Connect(Component)". Either wrap the root component in a <Provider>, or pass a custom React context provider to <Provider> and the corresponding React context consumer to Connect(Component) in connect options.

This error is actually not error, it will work ok after adding provider, etc...

https://codesandbox.io/s/1027585xjl

react-redux v7 - Error: The root route must render a single element

This is react-router error caused by the React.isValidElement call.

What is the expected behavior?

Not returning the error

Which versions of React, ReactDOM/React Native, Redux, and React Redux are you using? Which browser and OS are affected by this issue? Did this work in previous versions of React Redux?

React, ReactDOM - 16.8.4
Redux - 4.0.1
React Redux - 7.0.0-beta.0

@amertak amertak changed the title React-router@3 with v7 beta not working React-router@3 with v7 beta Mar 22, 2019
@markerikson
Copy link
Contributor

My first response is that this is presumably a React-Router bug, not a React-Redux bug. We now use React.memo() as part of our connected wrapper component, and it looks like React-Router isn't handling that correctly.

In fact, if I change your example to:

const Memoized = React.memo(Component);

const App = () => (
  <Router history={hashHistory}>
    <Route path="*" component={Memoized } />
  </Router>
);

I see the same error.

So, this needs to be fixed in React-Router somehow.

@timdorr
Copy link
Member

timdorr commented Mar 22, 2019

Yes, this is a Router bug. We don't maintain 3.x any longer, so you'll need to migrate to 5.0 (which has support for React.memo/React.lazy components).

@dlong500
Copy link

@timdorr That's quite unfortunate given that it is no simple migration (especially if you are using SSR). Can you point to any thorough guides for migrating that cover more than just the basics?

@stevemckenzie
Copy link

@dlong500 I just forked it the other day to address this issue as the application I'm working on cannot easily upgrade react-router.

I'm not sure if this was the best approach but the change is minimal. We use RouterContext and I only needed to address 1 condition. The other part was just simply upgrading the version of react and react-dom that react-router used.

Here's my commits: https://github.com/stevemckenzie/react-router/commits/v3

1 commit is just adding the build files to make life a little easier with a fork..

@stevemckenzie
Copy link

@timdorr it would be great if you could give your opinion about my condition change above 🙃

@Pegase745
Copy link

@timdorr I just noticed a recent release of react-router@3.2.3. Would you think it's possible to address this react-redux issue in a new release as proposed @stevemckenzie above ?

@dlong500
Copy link

dlong500 commented Jul 10, 2019

@Pegase745 The latest version of react-redux works fine now with react-router 3.2.3. The "bug" was fixed in react-router. There is no change needed in react-redux. See remix-run/react-router#6806 for details.

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

No branches or pull requests

6 participants