-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Either re-export all from Redux or make a peerDependency #238
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
Comments
I definitely want it to be a direct dependency, not a peer. The point is that you should only need to add RSK to your own In theory, if RSK has been added to a project, you don't need to have a specific listed dependency on Redux itself, and you can import from in. In practice, this may sorta just be because Yarn and NPM do hoist dependencies. I'll agree that we might want to go ahead and re-export everything from Redux as well. One other small downside is that React-Redux says it wants Redux as a peer, and logs a warning if only RSK is installed. |
What about immer? https://github.com/immerjs/immer/releases/tag/v5.0.0 Immer just released v5. |
@bugzpodder : yes, although it would probably be a point release since that's non-breaking. However, note that RSK's serialization check middleware will specifically throw errors by default if you try to put non-serializable values like Maps and Sets into the Redux store, so I don't see that being much benefit here anyway. |
@bugzpodder : in addition, it looks like https://bundlephobia.com/result?p=immer@5.0.0 There's an issue at immerjs/immer#449 to discuss trying to shrink things a bit, but given that RSK won't benefit from those features, I'm going to avoid updating Immer for now. |
no problem, I don't need it. I did find in my app that we were storing Map in a few places and I had to manually turn the warning off. |
Yeah, that should be configurable via https://redux-starter-kit.js.org/api/getdefaultmiddleware#api-reference |
Fixed by #243 . |
and now out in https://github.com/reduxjs/redux-starter-kit/releases/tag/v1.0.1 . |
It would be helpful for us to have redux as a peer dependency because we do use redux and react-redux today and we can't really switch redux to redux-toolkit at once in our codebase. So we want to try out redux-toolkit gradually but when we end up with two versions of Redux which is not really nice. |
@hornta we re-export all of redux. What are you missing? https://github.com/reduxjs/redux-toolkit/blob/master/src/index.ts#L2 |
@phryneas Nevermind, yea that's true! :) mostly a refactor of our require('redux') to require('@reduxjs/toolkit') I guess. |
👋 Hello! I suspect that
redux
may need to be apeerDependency
, but first I'll describe the issue I see:I have two copies of
redux
in my tree, andredux-starter-kit
is being supplied its own version.Why do I depend on
redux
if I useredux-starter-kit
? Well, some of my hooks do stuff like:So they use
bindActionCreators
, which is exported fromredux
and notredux-starter-kit
.Regardless of this, I suspect that
redux
may need to be apeerDependency
anyway, because other things declare it as apeerDependency
, and if you rely on it to be supplied byredux-starter-kit
then you have no guarantee what version you will get – it's not guaranteed thatredux-starter-kit
's version will be the one hoisted to the top of thenode_modules
tree.The text was updated successfully, but these errors were encountered: