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

New feature: $createObservableFunction #48

Merged
merged 4 commits into from
Jul 28, 2017

Conversation

regou
Copy link
Collaborator

@regou regou commented Jul 14, 2017

Convert function calls to observable sequence which emits the call arguments.
This make vue-rx become more collaborative with components that using callbacks

@regou regou requested a review from yyx990803 July 14, 2017 10:57
@regou regou self-assigned this Jul 14, 2017
@regou
Copy link
Collaborator Author

regou commented Jul 25, 2017

@yyx990803 It's been a while... Will you review this or should I merge directly ?
My fork is already on our production environment.
It'll be great if the community able to use it after the next release.

The usage is like this:
In template, normal callback
In constructor.
Maybe can save you some time.

@yyx990803
Copy link
Member

@regou thanks for the PR! This one slipped through the notifications.

I've got some feedback regarding the API: when I see a method being called in the template, I tend to look for it inside the methods block, but since it's registered imperatively, it can technically appear anywhere (lifecycle hooks, subscriptions function...) So this makes it less obvious to locate where a method is defined.

Maybe similar to the domStreams option, we can have an observableMethods option that makes it more declarative, which just calls $createObservableMethod internally (btw, $createObservableMethod seems more consistent with methods).

@regou
Copy link
Collaborator Author

regou commented Jul 26, 2017

@yyx990803 5c8f9c3
With simplified example

@yyx990803 yyx990803 merged commit bb0edcb into vuejs:master Jul 28, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants