artsy / react-redux-controller

Library for creating a controller layer to link React and Redux, on top of react-redux.
MIT License
97 stars 10 forks source link

Couple controllers to selectors using `this` #15

Open acjay opened 7 years ago

acjay commented 7 years ago

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.

plandem commented 7 years ago

any news? :)

acjay commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@acjay I see, will check Freactal :)

plandem commented 7 years ago

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