Skip to content

Export createStructuredSelector #691

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

Closed
wants to merge 2 commits into from
Closed

Conversation

t1gor
Copy link

@t1gor t1gor commented Aug 12, 2020

Add createStructuredSelector to exported types as this allows not to install reselect manually

Add `createStructuredSelector` to exported types as this allows not to install `reselect` manually
@netlify
Copy link

netlify bot commented Aug 12, 2020

Deploy preview for redux-starter-kit-docs ready!

Built with commit 532f608

https://deploy-preview-691--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 532f608:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration

@t1gor
Copy link
Author

t1gor commented Aug 12, 2020

Guys, there seems to be a problem with latest TS version (on Travis - https://travis-ci.org/github/reduxjs/redux-toolkit/builds/717252824?utm_source=github_status&utm_medium=notification), but I don't think this is related to pull-request. Could you please take a look?

@phryneas
Copy link
Member

Yeah, the fix for that is already included in #637 - just ignore it for now, it's the upcoming TS RC anyways ;)

@t1gor
Copy link
Author

t1gor commented Aug 12, 2020

@phryneas ok :) could you please merge my PR then?

@phryneas
Copy link
Member

Not gonna blindly merge and currently at work ;)

Also, I'll wait what @markerikson has to say on this one.

@markerikson
Copy link
Collaborator

Appreciate the PR, but I'm actually going to say no to this one for now, for a couple reasons.

We're not entirely trying to blindly re-export everything from the libs we depend on. Admittedly, we do re-export everything form the Redux core, but that's because this is a wrapper around that core. We re-export some stuff from Immer.

I've never been keen on using createStructuredSelector, personally, for whatever reason. With the migration from connect/mapState to useSelector, I think there's a lot fewer use cases where you might even want to use it.

Strictly speaking, you can probably get away with doing import {createStructuredSelector} from "reselect" even if you didn't list it, due to how JS package managers work these days. But, it is really easy to add your own dependency on reselect if necessary.

@t1gor
Copy link
Author

t1gor commented Aug 13, 2020

We're not entirely trying to blindly re-export everything from the libs we depend on. Admittedly, we do re-export everything form the Redux core, but that's because this is a wrapper around that core. We re-export some stuff from Immer.

This is clear and understandable.

... personally, for whatever reason. ...

Your words, not mine. It's a library you're creating, right? So use-cases might differ. We use createStructuredSelector when we need to map a bunch of smaller selectors to a large chunk of state. This just makes things easier to read and maintain afterwards. Just saying.

Strictly speaking, you can probably get away with doing import {createStructuredSelector} from "reselect" even if you didn't list it, due to how JS package managers work these days. But, it is really easy to add your own dependency on reselect if necessary.

Well, yes, but that would result in a hidden dependency. In case you would then decide to switch from reselect to smth. else we would be in trouble.

In any case - thanks for creating a great library 👍

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 this pull request may close these issues.

3 participants