[FEATURE ember-routing-bound-action-name]

[rwjblue]

The {{action}} helper will now use a non-quoted parameter and perform a bound property lookup against the action's target at the time the event is triggered.

If a non-quoted param is used, but the property lookup returns undefined a helpful assertion is made, and the specified parameter is used as a plain string action name (thus preventing this from breaking anyones applications).

Also, cleans up a few unrelated non-quoted uses of {{action}} in the test suite.

Cleaned up the website of any unquoted {{action}} helper usage here: emberjs/website#1149.

Closes #3368.

[lukemelia]

Seems like this would be a breaking API change for anyone's app where a property path exists with the same name as an action if the developer used unquoted action helper paths. A safer path would be to deprecate unquoted action names and offer an opt-in to bound property paths. I am OK with either path, but want to be sure we are being true to our SemVer promise.

[rwjblue]

@lukemelia - Good point. I am unsure if this is a common pattern (action named the same as a property), but it would certainly cause problems with this as it stands now. The change would see that a string property exists for a non-quoted action param and simply use that.

Making this opt-in in would resolve this concern, at the expense of a potentially long-running flag.

If that is the route we decide to go, I think we could enable the opt-in behavior via the existing feature flags by leaving the value null in the beta and stable branches features.json (a null value leaves the flags in the final build). This seems like a much better option that adding another mechanism (using an ENV.BOUND_ACTION_PARAM like we've done in the past).

Either way I believe that this shouldn't be a BUGFIX beta, so I have feature flagged it as ember-routing-bound-action-name.

[machty]

I'm somewhat on the fence about this change, though my objections might be picking some serious nits.

This change removes one layer of indirection at the expense of adding a binding and potentially making things less refactorable. Right now, you can fire a static string action and branch within the controller/route that handles that action (or have that handler translate that action into another one that it fires). This change makes it possible to put that branching logic into a controller and have the template binding choose which action to send. If this is a common use case and surprises folk for not being in the API already, then we should add it, but it does add more code and ties this action-firing behavior to a very specific controller, which is arguably less refactorable than having a dumber template that isn't so directly tied to a controller and letting the hierarchy more fully deciding the branching logic.

Picking serious nits, but figured I'd share in case it resonates.

[rwjblue]

@machty - I agree with you in principle, but I think the issue is larger than just with the {{action}} helper. We have stressed many times the difference between quoted and unquoted parameters in Handlebars is significant, but in at least a few cases we do not discriminate between them (see #3944 for another example). Currently, we allow unbound params to {{action}} which gives the impression that it'll do bound property lookup, but in fact it doesn't.

IMHO, we have options (possibly more than these though):

1. Deprecate the quoteless usage (I will do an audit of the other helpers to ensure each get a nice deprecation warning), leave the functionality alone (not adding bound param lookup), and add an example to the API docs and/or the guides to explain how to get this dynamic behavior.
2. Add bound property lookup, with a reasonable warning if it results in something unexpected (which is what I have done here).

I am totally fine either way, and can submit another PR handling the first case while we decide on the second.

[lukemelia]

I agree with señor Jackson. In this case, consistency trumps architecture API vinegar. 

—
Sent from Mailbox for iPhone

On Mon, Dec 16, 2013 at 9:13 AM, Robert Jackson notifications@github.com
wrote:

@machty - I agree with you in principle, but I think the issue is larger than just with the {{action}} helper. We have stressed many times the difference between quoted and unquoted parameters in Handlebars is significant, but in at least a few cases we do not discriminate between them (see #3944 for another example). Currently, we allow unbound params to {{action}} which gives the impression that it'll do bound property lookup, but in fact it doesn't.
IMHO, we have options (possibly more than these though):

1. Deprecate the quoteless usage (I will do an audit of the other helpers to ensure each get a nice deprecation warning), leave the functionality alone (not adding bound param lookup), and add an example to the API docs and/or the guides to explain how to get this dynamic behavior.
2. Add bound property lookup, with a reasonable warning if it results in something unexpected (which is what I have done here).

I am totally fine either way, and can submit another PR handling the first case while we decide on the second.

Reply to this email directly or view it on GitHub:
#3936 (comment)

[ebryn]

I agree with @rjackson and @lukemelia.

[machty]

👍 @rjackson could you rebase?

[rwjblue]

@machty - Done!

[machty]

I think we've already done this but let's quadruple check that there's not any hangover docs that document the old unquoted way.

[rwjblue]

I updated the website repo (just ack'ed for {{action and verified correct usage), I will do the same both in Ember itself and in the website repo to be sure.

[tomdale]

After discussing this at the core team meeting, we are in favor of merging this in. However, despite publicly stating pre-1.0 that "quoted"/unquoted identifiers in Handlebars templates may be considered meaningful, there are a significant number of apps that rely on the old behavior where they are treated identically.

In order to ease updating, we would like to have at least one stable release before this gets merged in that warns if unquoted identifiers are used. Before this feature is merged, we know that 100% of uses of unquoted action names are incorrect. Once this gets merged in, we will be unable to disambiguate.

Let's get a PR for a deprecation warning for when {{action}} uses unquoted action names, and once we have a stable release with that warning in, we can merge this PR.

[rwjblue]

The deprecation has been made in #4276, and merged into the 1.4 beta series in 2ef00a2.

#4285 will enable the feature going forward in preparation for the 1.5 beta series.

[ebryn]

This is a go for 1.4 beta