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

Either re-export all from Redux or make a peerDependency #238

Closed
exogen opened this issue Oct 29, 2019 · 11 comments
Closed

Either re-export all from Redux or make a peerDependency #238

exogen opened this issue Oct 29, 2019 · 11 comments

Comments

@exogen
Copy link
Contributor

exogen commented Oct 29, 2019

👋 Hello! I suspect that redux may need to be a peerDependency, but first I'll describe the issue I see:

$ yarn list redux
yarn list v1.19.0
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ redux-starter-kit@0.5.1
│  └─ redux@4.0.1
└─ redux@4.0.4
✨  Done in 0.82s.

I have two copies of redux in my tree, and redux-starter-kit is being supplied its own version.

Why do I depend on redux if I use redux-starter-kit? Well, some of my hooks do stuff like:

const dispatch = useDispatch();
const actions = useMemo(() =>
  bindActionCreators({ fooAction, barAction }, dispatch),
  [dispatch]
);

So they use bindActionCreators, which is exported from redux and not redux-starter-kit.

Regardless of this, I suspect that redux may need to be a peerDependency anyway, because other things declare it as a peerDependency, and if you rely on it to be supplied by redux-starter-kit then you have no guarantee what version you will get – it's not guaranteed that redux-starter-kit's version will be the one hoisted to the top of the node_modules tree.

@markerikson
Copy link
Collaborator

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 package.json, and get all the main pieces automatically (Redux, Immer, Reselect, thunk, etc).

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.

@bugzpodder
Copy link

What about immer?

https://github.com/immerjs/immer/releases/tag/v5.0.0 Immer just released v5.
If I want to use its new features (eg Map/Set drafts) I would need to wait for RSK to upgrade to 2.0?

@markerikson
Copy link
Collaborator

@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.

@markerikson
Copy link
Collaborator

@bugzpodder : in addition, it looks like immer@5.0 has a noticeable size increase:

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.

@bugzpodder
Copy link

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.

@markerikson
Copy link
Collaborator

Yeah, that should be configurable via getDefaultMiddleware:

https://redux-starter-kit.js.org/api/getdefaultmiddleware#api-reference

@markerikson
Copy link
Collaborator

Fixed by #243 .

@markerikson
Copy link
Collaborator

@hornta
Copy link

hornta commented Mar 31, 2021

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.

@phryneas
Copy link
Member

@hornta we re-export all of redux. What are you missing?

https://github.com/reduxjs/redux-toolkit/blob/master/src/index.ts#L2

@hornta
Copy link

hornta commented Mar 31, 2021

@phryneas Nevermind, yea that's true! :) mostly a refactor of our require('redux') to require('@reduxjs/toolkit') I guess.

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

5 participants