-
-
Notifications
You must be signed in to change notification settings - Fork 4.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
Implement user-helpers resolution #12917
Conversation
For now, I only ported two tests to show that it works.
97abbc2
to
df2cb52
Compare
@@ -23,36 +23,6 @@ QUnit.module('ember-htmlbars: custom app helpers', { | |||
} | |||
}); | |||
|
|||
QUnit.test('dashed shorthand helper is resolved from container', function() { |
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.
Replaced by "it can resolve custom helpers"
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.
At first I thought the "dashed" in the name implies that we have another "test matrix" situation here, but turns out literally all the tests in this file is prefixed with "dashed"
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.
yes, we used to have a distinction between dashed and non-dashed, but that was removed in 1.13.
registerHelper(name, funcOrClassBody) { | ||
let type = typeof funcOrClassBody; | ||
|
||
if (type === 'function') { |
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.
I think we should detect instances of Ember.Helper
and throw before this. We do not support Ember.Helper.extend()
style helpers, but typeof Ember.Helper.extend() === 'function'
.
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.
Implement user-helpers resolution
For now, I only ported two tests to show that it works.