Skip to content

Undocumented selectors functionality in createSlice #51

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
mattkahl opened this issue Dec 6, 2018 · 5 comments
Closed

Undocumented selectors functionality in createSlice #51

mattkahl opened this issue Dec 6, 2018 · 5 comments

Comments

@mattkahl
Copy link

mattkahl commented Dec 6, 2018

Thanks for the great tool! It uncovers some great existing utilities, provides some new ones, and sheds light on some useful idioms.

I had a question regarding createSlice. Having used a "ducks" structure previously, this was a great thing to uncover. And, naturally, I wondered why there wasn't an ability to also pass selectors into createSlice. However, I noticed that there is the ability to pass selectors into createSlice but it's undocumented. My only complaint about the functionality as it stands is that it modifies the name of selector. I understand why that happens for actions (and it makes it more readable to boot!), but I don't quite understand why that is needed for selectors.

So, two questions:

  1. Are you planning on exposing this functionality in the documentation? Or is it hidden for a reason?
  2. Would you be open to either (a) changing this functionality or (b) exposing createSliceSelector for import?

Thanks!

@markerikson
Copy link
Collaborator

The original inspiration for this is https://github.com/ericelliott/autodux , which has some more complex functionality in its createSlice implementation. Our version was originally written by @neurosnap , per discussion in #17 .

I'd absolutely like to make both the library and the slice/selector functionality more useful, but I've been focused on doing docs build system improvements. (That actually included setting up a new docs site for redux-starter-kit, at https://redux-starter-kit.js.org, which I just put up last night.)

So, the answers here are that we just haven't poked at it enough, and any stuff that's not documented is because we missed documenting it :)

If you've got specific suggestions, please tell us (and file a PR if possible). Also see #41 for some related discussion.

@mattkahl
Copy link
Author

mattkahl commented Dec 6, 2018

Thanks! Will do.

Looking at this more closely, it's not operating exactly the way I assumed.

I'll close now and maybe/hopefully open a PR after doing some battle-testing myself on a current project.

My thinking was that some toString niceties would be added via createSelector (a lá createAction) and an array of selectors could be passed to createSlice. All of those selectors would be wrapped via a function that passes in only the sliced state to the selector. Then they could be accessed via todosSlice.selectors.getCompleted(store.getState()). There are definitely some downsides to this approach (unnamed functions, reliance on createSelector, etc.).

I now see that I extrapolated the current implementation into something it wasn't. Realizing that the intention would be to use the generated slice selector when invoking a specific selector getCompleted(todosSlice.selectors.getTodos()).

@markerikson
Copy link
Collaborator

Just removed the slice selectors functionality entirely in #193 , on the grounds that it was useless. See #91 for more discussion.

@Letrab
Copy link

Letrab commented Dec 3, 2019

FYI: There is still a reference to it in the README.md:

A createSlice() function that accepts a set of reducer functions, a slice name, and an initial state value, and automatically generates corresponding action creators, types, and simple selector functions.

@markerikson
Copy link
Collaborator

Thanks, fixed.

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

3 participants