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

Couple controllers to selectors using this #15

Open
acjay opened this issue Mar 2, 2017 · 7 comments
Open

Couple controllers to selectors using this #15

acjay opened this issue Mar 2, 2017 · 7 comments

Comments

@acjay
Copy link
Contributor

acjay commented Mar 2, 2017

Building on #14, I believe it's possible to eliminate the generator from RRC.

Right now, controllers rely on generators to resolve promises and to get access to the selectors and dispatch. They use this to access each other. A dance happens in RRC to convert those controller generator methods into vanilla methods.

The reason generators are used is that it allows us to retrieve a "property getter" function, which will return the latest version of the selectors. If the selectors were simple properties, they could be stale in callbacks within the controllers. I didn't want to wrap properties in functions so that they would be expliticly called to retrieve their values at time of use. I'm not sure I was aware of Javascript's getter mechanism, which allows for this without requiring the caller to explicitly invoke them. That inspired #14.

Once that is done, all that's left to do is efficiently get all of these getters and dispatch on the controller instance. This seems extremely doable, and it's just a matter of choosing the right mechanism (mixin, Object.assign, prototype, whatever). And then, we'd just let the caller choose their preferred async mechanism, rather than baking co into RRC.

@acjay acjay changed the title Controller coupling to selectors using this Couple controllers to selectors using this Mar 2, 2017
@plandem
Copy link

plandem commented Apr 21, 2017

any news? :)

@acjay
Copy link
Contributor Author

acjay commented Apr 24, 2017

@plandem Embarrassingly -- no. We've been pretty well distracted from doing any core RRC work over at Artsy, since it basically works for our needs. So while a reworking of it would certainly make me feel better, it's not really blocking any functional changes we're looking to make.

But I'm interested in whether you've got any thoughts about this idea and #14.

@plandem
Copy link

plandem commented Apr 24, 2017

@acjay about blocking - at my pull request there is fix to handle exceptions at:

export function runControllerGenerator(propsGetter) {
.....
try {
       toController = yield value;
} catch (e) {
       gen.throw(e);
}

without this one I could not catch exceptions.

About #14, not sure about decorators, but getters/this and rest ideas - sounds cool :)

@plandem
Copy link

plandem commented Jun 21, 2017

@acjay

Ok, looks like noone has time here to move forward, so here is my result based on this issue and #14

https://github.com/plandem/react-redux-controller/tree/v2

I still in process of migrating examples, was busy with tests, still need more tests to cover integration with React and Redux. And probably it would be nice to have some performance checking.

@acjay
Copy link
Contributor Author

acjay commented Jun 21, 2017

We simply haven't had the cycles to test or iterate on React Redux Controller lately. For now, I might suggest either maintaining a hard fork or checking out https://github.com/FormidableLabs/freactal. Freactal seems to be a lot like what I'd want to make RRC, if we continued to iterate on it. But if we had the time, we'd probably strongly consider trying out switching to Freactal, to take advantage of its wider usage.

Sorry for leaving you hanging.

@plandem
Copy link

plandem commented Jun 21, 2017

@acjay I see, will check Freactal :)

@plandem
Copy link

plandem commented Jun 21, 2017

@acjay well, I checked Freactal, it needs more examples, for sure. And right now I don't see any advantages in comparison with controller's way(maybe I miss something) - the only disadvantage of controller is usage of context (so we are using state, props and context same time). So probably I will stick to hard fork - like name react-redux-controller, but with v2 actually there is nothing from original code base.

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

2 participants