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

support custom function for actions #116

Closed
wants to merge 1 commit into from

Conversation

angus-c
Copy link

@angus-c angus-c commented Nov 7, 2014

Since we have the benefit of functors for action objects, it would be great to allow custom function support, particularly for debugging, logging and profiling.

@spoike
Copy link
Member

spoike commented Nov 8, 2014

You may do this through the preEmit hook on actions instead. That way you don't need to remind yourself to do this.trigger(...).

myCategory.onMyAction.preEmit = function() {
    ga.send('send', 'event', 'My Category', 'My Action');  
};

It is the perfect spot for analytics (like GA) and benchmarking. Not sure about profiling (considering browsers nowadays have profiling tools).

Not sure if this PR is useful. I'd love to see use-cases if there are any before I decide to close it.

@jfsiii
Copy link

jfsiii commented Nov 10, 2014

@spoike, @simenbrekken listed a use case in #57 (comment)

Would perhaps be better with Reflux.createAction(fn).

It also seems to support the use case outlined in #58

I'd definitely prefer to hand createAction a function (which could be a simple function, an promise chain, etc) and have it converted into an action than to know about the preEmit property. IMO, it seems like the same behavior with a smaller API surface area.

@rymohr
Copy link
Contributor

rymohr commented Nov 25, 2014

I'd also prefer a custom function over preEmit and gave a use case in #103 (comment)

@spoike spoike mentioned this pull request Jan 4, 2015
@LongLiveCHIEF
Copy link

I'm going to close this PR, but leave issue #173 open, and open a new issue for the 0.3.0 milestone.

The reason primarily being that how this is implemented needs to be thought through and planned out for a wider range of needs/requirements. As async begins to grow more popular, it's more and more likely that sequences/behaviors would have similar requirements, and I believe implementing this feature now, in this way would cause too much breakage down the road when it's possibly implemented in a more dynamic approach, or via the use of middleware.

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.

5 participants