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

Implement utility selectors bound to current state #5

Merged
merged 2 commits into from Nov 5, 2019

Conversation

devinivy
Copy link
Contributor

Resolves #2. In short, the idea is: app.actions is to app.dispatch as app.selectors is to app.select. For former binds actions to dispatch(), and the latter binds getState() to selectors.

@devinivy devinivy added the enhancement New feature or request label Oct 28, 2019
map(value) :
(value && typeof value === 'object') ?
internals.mapEachLeafFunction(value, map) :
value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, huh. what cases does returning plain value cover? is that kinda a selector to access a constant?
less a question about this PR, more about redux practice generally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't design it for any specific use-case. I wanted select to be identical to selectors except with any functions in there bound to state, and that lead to me just ignoring unfamiliar values. I suppose the other option would be to strip them out for select, but it seems like it would make app.select and app.selectors less "reflective" of each other.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, gotcha, that makes sense. thanks for explaining! 🙏


return internals.mapEachLeafFunction(selectors, (selector) => {

return (...args) => selector(getState(), ...args);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

baller! 🍺 ❤️

@zemccartney
Copy link
Contributor

zemccartney commented Oct 28, 2019

random side-thought (taking back my "only" comment 🎺 ): maybe, in the example application, show an example of composing selectors? Not directly relevant to this, just came to mind on the topic of selectors in general. squirrel braining away!

@devinivy
Copy link
Contributor Author

Not sure I can picture what you're thinking of, although it reminds me of some things reselect allows you to do. Can you give an example of what you mean?

@zemccartney
Copy link
Contributor

ha, yea, it came to mind when I was reviewing the section in the redux docs on selectors and reselect as I was reviewing this, trying to better wrap my head around this.

But yea, for an example, something like:

const app = MiddleEnd.create({
        mods: {
            counter: {
                selectors: {
                    get: ({ counter }) => counter,
                    // Would this work?
                    cone: (inc) => app.select.counter.get() + inc
                }
        }
});

And I'm realizing now that that might not work, binding app in there like that...hmm. But yea, that's the gist of what i had in mind. No idea how useful, if functional, though 🤷‍♂

@zemccartney
Copy link
Contributor

ay, actually, on the topic of reselect, what are your thoughts on supporting (down the line, not here, of course) any sort of selector memoization, whether internally or via compatibility with a tool like reselect?

@devinivy
Copy link
Contributor Author

I would recommend using reselect for that. I don't think strange-middle-end needs to do anything special to support it. I am working on a separate library as time allows that implements selectors on normalizr entities (https://github.com/devinivy/denormie), which should work well with this library if it's ever completed. And yeah, your example above should work fine!

@zemccartney
Copy link
Contributor

ooooh very cool, thanks for the early notice. Pumped to see what comes out of that 🙏

Copy link

@bmleight bmleight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dig it!

@tylerconstance tylerconstance merged commit 305b9be into master Nov 5, 2019
@tylerconstance tylerconstance deleted the bound-selectors branch November 5, 2019 16:27
@tylerconstance tylerconstance added this to the 0.2.0 milestone Nov 6, 2019
@tylerconstance tylerconstance self-assigned this Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offer utility selectors bound to current state
4 participants