-
Notifications
You must be signed in to change notification settings - Fork 18
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
Custom events should only be subscribed to, when a listener exists #37
Comments
Good suggestion. However, doing that means we have to either ignore the changes to event listener props after mounted, or handle the subscription and disposal with I think I need some help here. We can put You can also check ES6 version by checking out branch template-component. |
I see what you are saying. I believe, there are actually two different use-cases for cycle-react components:
The current cycle-react implementation serves the first use-case very well. Prop updates are internally rewired to Observables, custom event updates coming from observables can be listened to using normal callback functions. For the second use-case I would actually prefer to interact with the cycle-react component in a purely reactive way. This means the following things:
In this second use-case I believe there is no necessity to ever update the props of the component from the outside after passing in the initial props. Observables are passed in for the input and thus don't need updating. Also in a reactive world I dont' really see a point in changing the event listener prop - necessary behaviour changes can be handled within the chain of Observables. I am not really sure of the best way to implement this. Should the cycle-react API provide two types of components or should this be handled internally? If we have two types of components, we could certainly share a lot of the code... When handling it internally, I see the following changes:
What do you think? Do you agree with those two use-cases and if so, how would you implement it: additional API or internal changes? I am also not quite sure, where you are going with the template components... Could you please elaborate? |
First of all, thank you very much for your valuable input. And sorry I didn't respond immediately, I was thinking about what's the best solution. I agree with both use-cases. In fact, I think what makes cycle-react different from other libraries is the way we handle events, by separating events from PropsSubject. That being said, although I think we should probably change the name For example, I think this is a good design and close to what we're doing right now https://github.com/Day8/re-frame/blob/master/examples/simple/src/simple/core.cljs#L67-L73. I think there are two actions we can take:
Template component is my experiment of separating view out of observables. The name "template component" is not very good, so I will change that. However, I think it's a better way to write components. |
No problem. I prefer the thinking-approach ;)
Just to prevent a misunderstanding: I did not mean that const Component = Cycle.component( 'Component', ( interactions, props ) => {
// props.count and props.text are exactly the observables that where passed in
const view = Rx.Observable.combineLatest( props.count, props.text,
( count, text ) => <div> {text}:{count} </div> );
const someEvent = Rx.Observable.interval( 100 );
return { view, someEvent };
} );
function someFunction()
const sinks = Cycle.sinks();
const count = Rx.Observable.interval( 1000 );
const text = Rx.Observable.just( 'foo' );
const someEventSink = sinks.get( 'someEventName' );
const subscription = someEventSink.subscribe( ev => console.log( 'Some event happened' );
return (
<Component
count={count} text={text} /* sources */
someEvent={sinks.forward( 'someEventName' )} /* sinks */
/>
);
} I introduced As I said - this would only be my imaginary solution for the second use-case. It also aims at a minimum alteration of the observables within
I can imagine people doing both, though I personally am also more interested in using
I see the point, but I personally disagree. I prefer the Cycle-approach of not having a centralized store or event-system. Though no one is stopping anyone from using redux or even
I think Sorry for my extensive response and thank you for your insights. |
Cool, I see your point. I think const Component = Cycle.component( 'Component', ( interactions, props ) => {
const view = Rx.Observable.combineLatest( props.count, props.text,
( count, text ) => <div> {text}:{count} </div> );
const someEvent = Rx.Observable.interval( 100 );
return {
view,
events: { someEvent }
};
} );
function someFunction()
const count = Rx.Observable.interval( 1000 );
const text = Rx.Observable.just( 'foo' );
const someEventSink = new Rx.Subject();
const subscription = someEventSink.subscribe( ev => console.log( 'Some event happened' );
return (
<Component
count={count} text={text} /* sources */
someEvent={someEventSink.onNext.bind(someEventSink)} /* sinks */
/>
); The problem is that the sink is not provided by the framework, so I was thinking about the application-wide solution. I think it's a good idea to introduce |
OK. Good to see that my ideas aren't too far off ;) Your example shows that what I intend is nearly possible, though it still has the problem of the original issue: subscribing to all custom events. Also there is currently no way of forwarding errors and completion of custom events in a reactive way. I think I have a working plan in my mind of how we could achieve both our visions and handle both use-cases. In general I think, Thanks again! |
Fixed in f80fb3c. The idea off working with custom events is tracked in the backlog https://github.com/pH200/cycle-react/projects/1 |
Better late than never... while working on a solution that supports the ideas I proposed in this comment, I ended up rewriting everything as I wanted to support RX5 (or any ES Observable compatible library) and made a move to ES7, etc. There isn't much left of your original code, even though it obviously was a big inspiration! You can find the repository in reactive-react-component (yes the name is weird, but my idea is to support other frameworks in a similar fashion). It is not yet on npm. Anyway. I hope it may be of use for you as well and maybe you even have further ideas or critique. Thanks again for your great library, which as I said, was the main source / inspiration! :) |
@FunkMonkey That's a really good job, and honestly I like the decision of not using some strange word for your library. TBH I think the most challenging part would be handling errors in the right way, and make it a lot simpler by using the framework. That's something I found recently, and I think maybe you will find it useful for your framework, too. |
Currently in create-react-class#L144 all custom events are being subscribed to, even if nobody ever listens to them. As a performance optimization, a custom event observable should only be subscribed to, when a listener is passed as a property.
p.s. Thanks for the great library. I'm using it in multiple projects...
The text was updated successfully, but these errors were encountered: