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-redux dependency bug #244

Closed
catamphetamine opened this issue Feb 21, 2019 · 15 comments · Fixed by #337
Closed

react-redux dependency bug #244

catamphetamine opened this issue Feb 21, 2019 · 15 comments · Fixed by #337

Comments

@catamphetamine
Copy link

catamphetamine commented Feb 21, 2019

found has a bug when it has react-redux added in its dependencies instead of making it a peerDependency.
This results in found connect()-ing to a different context and waiting for a <Provider store/> in that context but the <Provider/> that's instantiated in container is from a different node_module so it is therefore in an entirely different context and so found throws an error that <Provider store/> is not found in found's context.

For example, with such node_modules structure:

node_modules/found/node_modules/react-redux

Uncaught Invariant Violation: Could not find "store" in either the context or props of "ConnectedRouter". Either wrap the root component in a <Provider>, or explicitly pass "store" as a prop to "ConnectedRouter".
    at invariant (webpack:////.../node_modules/invariant/browser.js?:38:15)
    at new Connect (webpack:////.../node_modules/found/node_modules/react-redux/es/components/connectAdvanced.js?:132:57)
    at constructClassInstance (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:11222:18)
    at updateClassComponent (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:14346:5)
    at beginWork (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:15222:16)
    at performUnitOfWork (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:18789:12)
    at workLoop (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:18829:24)
    at HTMLUnknownElement.callCallback (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:149:14)
    at Object.invokeGuardedCallbackDev (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:199:16)
    at invokeGuardedCallback (webpack:////.../node_modules/react-dom/cjs/react-dom.development.js?:256:31)

The original bug reporter is @rcambrj , he can answer further questions.

@catamphetamine
Copy link
Author

@rcambrj Can you also check whether you're using react-redux@5 or react-redux@6?
If you have react-redux@6 in your app's package.json then it explains why found installed its own react-redux: because those are different (incompatible) versions of the library.

@rcambrj
Copy link

rcambrj commented Feb 21, 2019

Can confirm this is the problem (react-redux@6 instead of react-redux@5).

@catamphetamine
Copy link
Author

Ok.
I'm closing this issue for now, but I'm still certain that found having react-redux (and redux) in dependencies instead of peerDependencies is a bug and will be a source of confusion resulting in similar issues in future.

@taion
Copy link
Contributor

taion commented Feb 21, 2019

I'm not really sure what the best way to deal with this is, to be honest.

Ideally I don't want to force users to have to install react-redux and redux on their own, because it's possible to use Found without directly interacting with Redux.

The incompatibility is an issue, though. Not sure what the right answer is.

@rcambrj
Copy link

rcambrj commented Feb 21, 2019 via email

@taion
Copy link
Contributor

taion commented Feb 21, 2019

Hmm, I'm not sure how we'd do that though. Like check that our connect components aren't compatible with the Provider? But they don't connect...

@catamphetamine
Copy link
Author

@taion I guess so. If found has its own node_modules/react-redux then there's no way it could access the root-level react-redux@6 to inspect its version.

@rcambrj
Copy link

rcambrj commented Feb 21, 2019

I think I understand: the error is thrown before getting to found's logic.

Uncaught Invariant Violation: Could not find "store" in either the context or props of "ConnectedRouter". Either wrap the root component in a <Provider>, or explicitly pass "store" as a prop to "ConnectedRouter".

Right?

I suppose the prop expectations could be made looser, and a custom check made later on, it's a bit smelly though.

@taion
Copy link
Contributor

taion commented Feb 21, 2019

Right, but we don't know if that's because you've set up the wrong version of Redux, or if it's because you just didn't render a <Provider>.

@rcambrj
Copy link

rcambrj commented Feb 22, 2019

@taion a store was being provided via context as a result of wrapping with a <Provider>, it was just a react-redux@6 store, while found was expecting a react-redux@5 store.

@taion
Copy link
Contributor

taion commented Feb 22, 2019

Right, but it's not the same context API, right? React Redux v5 uses the old context API, while React Redux v6 uses the new one. They don't inter-operate as expected.

@smooth-opperator
Copy link

Having the same issue but using react-redux @5

@taion
Copy link
Contributor

taion commented Apr 5, 2019

If you're using React Redux v5, make sure you're still using the v0.3.x releases here.

@taion
Copy link
Contributor

taion commented Apr 12, 2019

on second thought, let's do make this a peer dep. things work with 5, 6, and 7 equally now that we're not longer hacking around stuff in the alphas

@taion
Copy link
Contributor

taion commented Apr 12, 2019

ugh, on third thought, this makes things quite clunky. i'm going to explicitly document this caveat instead and suggest some workarounds

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

Successfully merging a pull request may close this issue.

4 participants