-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use correct bound helper params #3410
Conversation
@@ -1,7 +1,7 @@ | |||
import Ember from 'ember'; | |||
|
|||
export function <%= camelizedModuleName %>(input) { | |||
return input; | |||
export function <%= camelizedModuleName %>(params, hash, options, env) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bound helpers only get params
and hash
(the first two args).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should likely put the args in comments (otherwise the JSHint tests will blow up after generating):
export function <%= camelizedModuleName %>(/* params, hash, options, env */) {
}
And I think that the generate tests will also need to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue I'm a little confused, all the helpers I see in core have 4 params. Can you point me to some documentation on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helpers in core are not using Ember.HTMLBars.makeBoundHelper
, since they are not bound helpers.
Documentation is here:
@rwjblue Does it look okay now? |
Use correct bound helper params
I think this will make the default helper test not pass: https://github.com/ember-cli/ember-cli/blob/master/blueprints/helper-test/files/tests/unit/helpers/__name__-test.js |
@kimroen - You are correct, mind submitting a PR? |
@rwjblue Done :) |
No description provided.