ember-composable-helpers icon indicating copy to clipboard operation
ember-composable-helpers copied to clipboard

Should/can `pipe` respect the action's `target` attribute?

Open nwhittaker opened this issue 8 years ago • 8 comments

Given:

{{action (pipe addToCart purchase redirectToThankYouPage) item target='checkoutService'}}

pipe() appears to resolve for addToCart(), purchase(), and redirectToThankYouPage() against the current component/controller space, rather than look in the target object -- checkoutService, in this case.

nwhittaker avatar Jun 20 '16 18:06 nwhittaker

Currently the target option will not do anything, but it should. Thanks for reporting, will fix!

Edit: PR is also welcome, I won't be able to get to this until a little bit later.

poteto avatar Jun 20 '16 18:06 poteto

@poteto what should target do here?

pipe is a pure helper, it doesn't know anything about it's context other than what's passed in to it. To make target work, we'd have to mess around with the stream.

The same could be accomplished with

{{action (pipe 
   (action "addToCart" target="checkoutService")
   (action "purchase" target="checkoutService")
   (action "redirectToThankYouPage" target="checkoutService")
) item}}

It's more verbose, but it'll do this without trickery.

Thoughts?

taras avatar Jul 27 '16 02:07 taras

I'm with @taras and :-1: on this one. The action keyword has much lower level access to the template context, which a Ember.Helper does not have. This is a limitation we should just deal with.

ghost avatar Jul 27 '16 14:07 ghost

pipe is a pure helper, it doesn't know anything about it's context other than what's passed in to it.

I'm finding that the outer action helper will call pipe in the context of its target parameter. You could then invoke the piped actions within the same context. However these piped actions would have to be invoked differently depending on whether they were passed to pipe as a string, closure or action-closure (others?): e.g. (action (pipe 'addToCart' purchase (action 'redirectToThankYouPage' target=cxt2)) target=cxt1).

To clarify the initial confusion, I think allowing pipe to pick up the outer action's positional parameters implies pipe is aware of, and adheres to, the action helper's interface. If the consensus is this request is feature creep, it might be worth at least clarifying the scope of the pipe helper's awareness in the documentation.

nwhittaker avatar Jul 27 '16 23:07 nwhittaker

I think allowing pipe to pick up the outer action's positional parameters implies pipe is aware of, and adheres to, the action helper's interface.

This is actually not accurate. An accurate way to think about this is to think of (pipe addToCart purchase redirectToThankYouPage) as pipe is a function that will invoke in sequence 3 functions that are passed into it. When pipe is invoked with a parameter, this parameter is passed to the first function. Result from the first function is passed as parameter to the second function and so on.

Parameters that are passed to action helper only apply to action helper. Action helper happens to pass these parameters to the function that's passed into it. There is no explicit binding between action and pipe, they just happen to work well together.

taras avatar Jul 28 '16 02:07 taras

This is actually not accurate.

@taras, understood this is not how it's actually implemented. I guess my claim is that the current implementation is less intuitive and leads to less DRY code. As a consumer of the library, it's more attractive for me to write (action (pipe addToCart purchase ...) target='cxt'). As a producer of the library, I'm sure (action (pipe (action addToCart target='cxt') (action purchase target='cxt') ...) is more attractive to maintain and support -- but it can end up adding verbosity and DRYness violations to user land code.

Action helper happens to pass these parameters to the function that's passed into it.

The action helper also happens to call the function that's passed into in the context of action helper's target attribute, but pipe swallows this detail instead of passing it on. IMO, it's not clear from the documentation what pipe is choosing to pass through from its calling context, be it positional parameters, named parameters, and/or the target object.

Granted this is likely an edge case, but just be aware, I guess, that it could be a possible source of confusion for others and either addressing it in the code, the docs, or via some sort of Ember warning might be worthwhile.

nwhittaker avatar Jul 29 '16 16:07 nwhittaker

I believe (pipe (action "foo) (action "bar") target=something) would solve @nwhittaker's use case without requiring the use of a keyword. We would then need to use call or apply with that context when invoking the action.

export default pipe([actions = [], { target }]) {
  if (target) {
    ... 
  }
}

poteto avatar Jul 30 '16 00:07 poteto

@poteto would this change target for closure actions that are bound to other objects?

For example,

<button {{action (pipe 
   passedInAction 
   (action "addToCart") 
   (action "purchase") target=checkoutService))
}}>Add to cart</button>

It seems that passedInAction will have checkoutService context.

To overcome this, you'd have to do something like this

<button {{action (pipe 
   passedInAction 
   (pipe (action "addToCart") (action "purchase") target=checkoutService))
}}>Add to cart</button>

I'm happy to create a PR for this if this doesn't add surprising behaviour to pipe.

taras avatar Jul 30 '16 10:07 taras