From 062465af97bfc8d913e58cd1e0cc84df7f8a6a67 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Wed, 3 Jun 2015 22:19:22 -0400 Subject: [PATCH 1/4] Only call deactivate when being deactivated --- packages/ember-metal/lib/streams/key-stream.js | 11 +++++++---- packages/ember-metal/lib/streams/stream.js | 8 ++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/ember-metal/lib/streams/key-stream.js b/packages/ember-metal/lib/streams/key-stream.js index d7c217018c2..2dc1927a873 100644 --- a/packages/ember-metal/lib/streams/key-stream.js +++ b/packages/ember-metal/lib/streams/key-stream.js @@ -59,7 +59,7 @@ merge(KeyStream.prototype, { var object = this.sourceDep.getValue(); if (object !== this.observedObject) { - this.deactivate(); + this._clearObservedObject(); if (object && typeof object === 'object') { addObserver(object, this.key, this, this.notify); @@ -70,13 +70,16 @@ merge(KeyStream.prototype, { _super$deactivate: Stream.prototype.deactivate, - deactivate() { - this._super$deactivate(); - + _clearObservedObject() { if (this.observedObject) { removeObserver(this.observedObject, this.key, this, this.notify); this.observedObject = null; } + }, + + deactivate() { + this._super$deactivate(); + this._clearObservedObject(); } }); diff --git a/packages/ember-metal/lib/streams/stream.js b/packages/ember-metal/lib/streams/stream.js index 9c317791a73..43a91a1c58f 100644 --- a/packages/ember-metal/lib/streams/stream.js +++ b/packages/ember-metal/lib/streams/stream.js @@ -180,7 +180,7 @@ Stream.prototype = { revalidate(value) { if (value !== this.observedProxy) { - this.deactivate(); + this._clearObservedProxy(); ProxyMixin = ProxyMixin || Ember.__loader.require('ember-runtime/mixins/-proxy').default; @@ -191,13 +191,17 @@ Stream.prototype = { } }, - deactivate() { + _clearObservedProxy() { if (this.observedProxy) { removeObserver(this.observedProxy, 'content', this, this.notify); this.observedProxy = null; } }, + deactivate() { + this._clearObservedProxy(); + }, + compute() { throw new Error("Stream error: compute not implemented"); }, From 334b3df0aff2038ceda0293fecebc1ee16cc8d54 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Mon, 25 May 2015 12:49:11 -0400 Subject: [PATCH 2/4] Implement Ember.Helper API --- package.json | 2 +- .../lib/system/application.js | 1 - .../ember-application/lib/system/resolver.js | 7 +- .../default_resolver_test.js | 53 ++- packages/ember-htmlbars/lib/helper.js | 25 ++ packages/ember-htmlbars/lib/hooks/element.js | 4 +- .../ember-htmlbars/lib/hooks/has-helper.js | 16 +- .../ember-htmlbars/lib/hooks/invoke-helper.js | 43 +-- packages/ember-htmlbars/lib/hooks/subexpr.js | 46 +-- packages/ember-htmlbars/lib/main.js | 3 + .../lib/streams/built-in-helper.js | 27 ++ .../lib/streams/compat-helper.js | 22 ++ .../lib/streams/helper-factory.js | 35 ++ .../lib/streams/helper-instance.js | 23 ++ packages/ember-htmlbars/lib/streams/utils.js | 21 + .../lib/system/invoke-helper.js | 17 + .../lib/system/lookup-helper.js | 47 +-- .../lib/system/make_bound_helper.js | 8 +- .../ember-htmlbars/lib/utils/is-component.js | 4 +- .../tests/compat/handlebars_get_test.js | 18 +- .../tests/compat/helper_test.js | 6 +- .../tests/compat/make-view-helper_test.js | 1 - .../tests/helpers/custom_helper_test.js | 360 ++++++++++++++++++ .../tests/helpers/unbound_test.js | 1 - .../ember-htmlbars/tests/helpers/view_test.js | 1 - .../tests/integration/block_params_test.js | 1 - .../integration/component_element_id_test.js | 1 - .../integration/component_invocation_test.js | 1 - .../integration/component_lifecycle_test.js | 1 - .../tests/integration/mutable_binding_test.js | 1 - .../void-element-component-test.js | 1 - .../tests/system/lookup-helper_test.js | 47 +-- .../tests/system/make_bound_helper_test.js | 3 +- .../tests/system/make_view_helper_test.js | 1 - packages/ember-views/lib/component_lookup.js | 7 +- .../tests/helpers/helper_registration_test.js | 34 +- 36 files changed, 702 insertions(+), 187 deletions(-) create mode 100644 packages/ember-htmlbars/lib/helper.js create mode 100644 packages/ember-htmlbars/lib/streams/built-in-helper.js create mode 100644 packages/ember-htmlbars/lib/streams/compat-helper.js create mode 100644 packages/ember-htmlbars/lib/streams/helper-factory.js create mode 100644 packages/ember-htmlbars/lib/streams/helper-instance.js create mode 100644 packages/ember-htmlbars/lib/streams/utils.js create mode 100644 packages/ember-htmlbars/lib/system/invoke-helper.js create mode 100644 packages/ember-htmlbars/tests/helpers/custom_helper_test.js diff --git a/package.json b/package.json index 7ac40e55e21..87e8afcf3df 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "express": "^4.5.0", "github": "^0.2.3", "glob": "~4.3.2", - "htmlbars": "0.13.25", + "htmlbars": "0.13.28", "qunit-extras": "^1.3.0", "qunitjs": "^1.16.0", "route-recognizer": "0.1.5", diff --git a/packages/ember-application/lib/system/application.js b/packages/ember-application/lib/system/application.js index 2dd9c9ca1ac..2b7248351c5 100644 --- a/packages/ember-application/lib/system/application.js +++ b/packages/ember-application/lib/system/application.js @@ -1016,7 +1016,6 @@ Application.reopenClass({ registry.optionsForType('component', { singleton: false }); registry.optionsForType('view', { singleton: false }); registry.optionsForType('template', { instantiate: false }); - registry.optionsForType('helper', { instantiate: false }); registry.register('application:main', namespace, { instantiate: false }); diff --git a/packages/ember-application/lib/system/resolver.js b/packages/ember-application/lib/system/resolver.js index 10ce9843160..da8109d81e3 100644 --- a/packages/ember-application/lib/system/resolver.js +++ b/packages/ember-application/lib/system/resolver.js @@ -15,6 +15,7 @@ import EmberObject from 'ember-runtime/system/object'; import Namespace from 'ember-runtime/system/namespace'; import helpers from 'ember-htmlbars/helpers'; import validateType from 'ember-application/utils/validate-type'; +import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; export var Resolver = EmberObject.extend({ /* @@ -374,7 +375,11 @@ export default EmberObject.extend({ @public */ resolveHelper(parsedName) { - return this.resolveOther(parsedName) || helpers[parsedName.fullNameWithoutType]; + var resolved = this.resolveOther(parsedName) || helpers[parsedName.fullNameWithoutType]; + if (resolved && !resolved.isHelperFactory && !resolved.isHelperInstance && !resolved.isHTMLBars && typeof resolved === 'function') { + resolved = new HandlebarsCompatibleHelper(resolved); + } + return resolved; }, /** Look up the specified object (from parsedName) on the appropriate diff --git a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js index 0bde4b6f075..b09c4e7a573 100644 --- a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js +++ b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js @@ -9,6 +9,11 @@ import Service from "ember-runtime/system/service"; import EmberObject from "ember-runtime/system/object"; import Namespace from "ember-runtime/system/namespace"; import Application from "ember-application/system/application"; +import Helper from "ember-htmlbars/helper"; +import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; +import makeHandlebarsBoundHelper from "ember-htmlbars/compat/make-bound-helper"; +import makeViewHelper from "ember-htmlbars/system/make-view-helper"; +import makeHTMLBarsBoundHelper from "ember-htmlbars/system/make_bound_helper"; import { registerHelper } from "ember-htmlbars/helpers"; @@ -102,12 +107,48 @@ QUnit.test("the default resolver resolves helpers", function() { }); QUnit.test("the default resolver resolves container-registered helpers", function() { - function gooresolvertestHelper() { return 'GOO'; } - function gooGazResolverTestHelper() { return 'GAZ'; } - application.register('helper:gooresolvertest', gooresolvertestHelper); - application.register('helper:goo-baz-resolver-test', gooGazResolverTestHelper); - equal(gooresolvertestHelper, locator.lookup('helper:gooresolvertest'), "looks up gooresolvertest helper"); - equal(gooGazResolverTestHelper, locator.lookup('helper:goo-baz-resolver-test'), "looks up gooGazResolverTestHelper helper"); + let shorthandHelper = Helper.helper(function() {}); + let helper = Helper.extend(); + + application.register('helper:shorthand', shorthandHelper); + application.register('helper:complete', helper); + + let lookedUpShorthandHelper = locator.lookupFactory('helper:shorthand'); + ok(lookedUpShorthandHelper.isHelperInstance, 'shorthand helper isHelper'); + + let lookedUpHelper = locator.lookupFactory('helper:complete'); + ok(lookedUpHelper.isHelperFactory, 'complete helper is factory'); + ok(helper.detect(lookedUpHelper), "looked up complete helper"); +}); + +QUnit.test("the default resolver resolves helpers on the namespace", function() { + let ShorthandHelper = Helper.helper(function() {}); + let CompleteHelper = Helper.extend(); + let LegacyBareFunctionHelper = function() {}; + let LegacyHandlebarsBoundHelper = makeHandlebarsBoundHelper(function() {}); + let LegacyHTMLBarsBoundHelper = makeHTMLBarsBoundHelper(function() {}); + let ViewHelper = makeViewHelper(function() {}); + + application.ShorthandHelper = ShorthandHelper; + application.CompleteHelper = CompleteHelper; + application.LegacyBareFunctionHelper = LegacyBareFunctionHelper; + application.LegacyHandlebarsBoundHelper = LegacyHandlebarsBoundHelper; + application.LegacyHtmlBarsBoundHelper = LegacyHTMLBarsBoundHelper; // Must use lowered "tml" in "HTMLBars" for resolver to find this + application.ViewHelper = ViewHelper; + + let resolvedShorthand = registry.resolve('helper:shorthand'); + let resolvedComplete = registry.resolve('helper:complete'); + let resolvedLegacy = registry.resolve('helper:legacy-bare-function'); + let resolvedLegacyHandlebars = registry.resolve('helper:legacy-handlebars-bound'); + let resolvedLegacyHTMLBars = registry.resolve('helper:legacy-html-bars-bound'); + let resolvedView = registry.resolve('helper:view'); + + equal(resolvedShorthand, ShorthandHelper, 'resolve fetches the shorthand helper factory'); + equal(resolvedComplete, CompleteHelper, 'resolve fetches the complete helper factory'); + ok(resolvedLegacy instanceof HandlebarsCompatibleHelper, 'legacy function helper is wrapped in HandlebarsCompatibleHelper'); + equal(resolvedView, ViewHelper, 'resolves view helper'); + equal(resolvedLegacyHTMLBars, LegacyHTMLBarsBoundHelper, 'resolves legacy HTMLBars bound helper'); + equal(resolvedLegacyHandlebars, LegacyHandlebarsBoundHelper, 'resolves legacy Handlebars bound helper'); }); QUnit.test("the default resolver throws an error if the fullName to resolve is invalid", function() { diff --git a/packages/ember-htmlbars/lib/helper.js b/packages/ember-htmlbars/lib/helper.js new file mode 100644 index 00000000000..6e77c4ae050 --- /dev/null +++ b/packages/ember-htmlbars/lib/helper.js @@ -0,0 +1,25 @@ +import Object from "ember-runtime/system/object"; + +// Ember.Helper.extend({ compute(params, hash) {} }); +var Helper = Object.extend({ + isHelper: true, + recompute() { + this._stream.notify(); + } +}); + +Helper.reopenClass({ + isHelperFactory: true +}); + +// Ember.Helper.helper(function(params, hash) {}); +export function helper(helperFn) { + return { + isHelperInstance: true, + compute: helperFn + }; +} + +Helper.helper = helper; + +export default Helper; diff --git a/packages/ember-htmlbars/lib/hooks/element.js b/packages/ember-htmlbars/lib/hooks/element.js index ddc63fd765e..d02fd31e61d 100644 --- a/packages/ember-htmlbars/lib/hooks/element.js +++ b/packages/ember-htmlbars/lib/hooks/element.js @@ -5,6 +5,7 @@ import { findHelper } from "ember-htmlbars/system/lookup-helper"; import { handleRedirect } from "htmlbars-runtime/hooks"; +import { buildHelperStream } from "ember-htmlbars/system/invoke-helper"; var fakeElement; @@ -32,7 +33,8 @@ export default function emberElement(morph, env, scope, path, params, hash, visi var result; var helper = findHelper(path, scope.self, env); if (helper) { - result = env.hooks.invokeHelper(null, env, scope, null, params, hash, helper, { element: morph.element }).value; + var helperStream = buildHelperStream(helper, params, hash, { element: morph.element }, env, scope); + result = helperStream.value(); } else { result = env.hooks.get(env, scope, path); } diff --git a/packages/ember-htmlbars/lib/hooks/has-helper.js b/packages/ember-htmlbars/lib/hooks/has-helper.js index 169f2b4f881..35d8e69ab10 100644 --- a/packages/ember-htmlbars/lib/hooks/has-helper.js +++ b/packages/ember-htmlbars/lib/hooks/has-helper.js @@ -1,5 +1,17 @@ -import { findHelper } from "ember-htmlbars/system/lookup-helper"; +import { validateLazyHelperName } from "ember-htmlbars/system/lookup-helper"; export default function hasHelperHook(env, scope, helperName) { - return !!findHelper(helperName, scope.self, env); + if (env.helpers[helperName]) { + return true; + } + + var container = env.container; + if (validateLazyHelperName(helperName, container, env.hooks.keywords)) { + var containerName = 'helper:' + helperName; + if (container._registry.has(containerName)) { + return true; + } + } + + return false; } diff --git a/packages/ember-htmlbars/lib/hooks/invoke-helper.js b/packages/ember-htmlbars/lib/hooks/invoke-helper.js index 9f72f98b8e2..92cd928c07d 100644 --- a/packages/ember-htmlbars/lib/hooks/invoke-helper.js +++ b/packages/ember-htmlbars/lib/hooks/invoke-helper.js @@ -1,43 +1,24 @@ import Ember from 'ember-metal/core'; // Ember.assert -import getValue from "ember-htmlbars/hooks/get-value"; +import { buildHelperStream } from "ember-htmlbars/system/invoke-helper"; +export default function invokeHelper(morph, env, scope, visitor, params, hash, helper, templates, context) { - -export default function invokeHelper(morph, env, scope, visitor, _params, _hash, helper, templates, context) { - var params, hash; - - if (typeof helper === 'function') { - params = getArrayValues(_params); - hash = getHashValues(_hash); - return { value: helper.call(context, params, hash, templates) }; - } else if (helper.isLegacyViewHelper) { + if (helper.isLegacyViewHelper) { Ember.assert("You can only pass attributes (such as name=value) not bare " + - "values to a helper for a View found in '" + helper.viewClass + "'", _params.length === 0); + "values to a helper for a View found in '" + helper.viewClass + "'", params.length === 0); - env.hooks.keyword('view', morph, env, scope, [helper.viewClass], _hash, templates.template.raw, null, visitor); + env.hooks.keyword('view', morph, env, scope, [helper.viewClass], hash, templates.template.raw, null, visitor); + // Opts into a special mode for view helpers return { handled: true }; - } else if (helper && helper.helperFunction) { - var helperFunc = helper.helperFunction; - return { value: helperFunc.call({}, _params, _hash, templates, env, scope) }; - } -} - -// We don't want to leak mutable cells into helpers, which -// are pure functions that can only work with values. -function getArrayValues(params) { - let out = []; - for (let i=0, l=params.length; i{{x-borf}} {{x-borf 'YES'}}"); + let helper = new HandlebarsCompatibleHelper(function(val) { + return arguments.length > 1 ? val : "BORF"; + }); - Ember.TEMPLATES.application = compile("
{{x-borf}} {{x-borf YES}}
"); - - boot(function() { - registry.register('helper:x-borf', function(val) { - return arguments.length > 1 ? val : "BORF"; - }); + boot(() => { + registry.register('helper:x-borf', helper); }); equal(Ember.$('#wrapper').text(), "BORF YES", "The helper was invoked from the container"); @@ -121,3 +122,24 @@ QUnit.test("Undashed helpers registered on the container can not (presently) be }); }, /A helper named 'omg' could not be found/); }); + +QUnit.test("Helpers can receive injections", function() { + Ember.TEMPLATES.application = compile("
{{full-name}}
"); + + var serviceCalled = false; + boot(function() { + registry.register('service:name-builder', Ember.Service.extend({ + build() { + serviceCalled = true; + } + })); + registry.register('helper:full-name', Ember.Helper.extend({ + nameBuilder: Ember.inject.service('name-builder'), + compute() { + this.get('nameBuilder').build(); + } + })); + }); + + ok(serviceCalled, 'service was injected, method called'); +}); From 415959d49c2b09a2e9160f63fc847a6c7d39f636 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Sun, 7 Jun 2015 20:39:40 -0400 Subject: [PATCH 3/4] Wrap ember-htmlbars-helper in a feature flag --- FEATURES.md | 5 +++++ features.json | 3 ++- .../dependency_injection/default_resolver_test.js | 6 +++--- packages/ember-htmlbars/lib/helper.js | 2 -- packages/ember-htmlbars/lib/main.js | 7 +++++-- .../ember-htmlbars/lib/system/make_bound_helper.js | 4 ++-- .../tests/helpers/custom_helper_test.js | 12 ++++++------ .../tests/system/lookup-helper_test.js | 4 ++-- .../ember/tests/helpers/helper_registration_test.js | 3 ++- 9 files changed, 27 insertions(+), 19 deletions(-) diff --git a/FEATURES.md b/FEATURES.md index fb0d67c5fc9..ca598244f45 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -310,3 +310,8 @@ for a detailed explanation. for each person.. E.g. a list of all `firstNames`, or `lastNames`, or `ages`. Addd in [#11196](https://github.com/emberjs/ember.js/pull/11196) + +* `ember-htmlbars-helper` + + Implements RFC https://github.com/emberjs/rfcs/pull/53, a public helper + api. diff --git a/features.json b/features.json index 4bb640ea38b..2f437d78411 100644 --- a/features.json +++ b/features.json @@ -20,7 +20,8 @@ "ember-routing-route-configured-query-params": null, "ember-libraries-isregistered": null, "ember-routing-htmlbars-improved-actions": true, - "ember-htmlbars-get-helper": null + "ember-htmlbars-get-helper": null, + "ember-htmlbars-helper": true }, "debugStatements": [ "Ember.warn", diff --git a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js index b09c4e7a573..f90ce7d490b 100644 --- a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js +++ b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js @@ -9,7 +9,7 @@ import Service from "ember-runtime/system/service"; import EmberObject from "ember-runtime/system/object"; import Namespace from "ember-runtime/system/namespace"; import Application from "ember-application/system/application"; -import Helper from "ember-htmlbars/helper"; +import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; import makeHandlebarsBoundHelper from "ember-htmlbars/compat/make-bound-helper"; import makeViewHelper from "ember-htmlbars/system/make-view-helper"; @@ -107,7 +107,7 @@ QUnit.test("the default resolver resolves helpers", function() { }); QUnit.test("the default resolver resolves container-registered helpers", function() { - let shorthandHelper = Helper.helper(function() {}); + let shorthandHelper = makeHelper(function() {}); let helper = Helper.extend(); application.register('helper:shorthand', shorthandHelper); @@ -122,7 +122,7 @@ QUnit.test("the default resolver resolves container-registered helpers", functio }); QUnit.test("the default resolver resolves helpers on the namespace", function() { - let ShorthandHelper = Helper.helper(function() {}); + let ShorthandHelper = makeHelper(function() {}); let CompleteHelper = Helper.extend(); let LegacyBareFunctionHelper = function() {}; let LegacyHandlebarsBoundHelper = makeHandlebarsBoundHelper(function() {}); diff --git a/packages/ember-htmlbars/lib/helper.js b/packages/ember-htmlbars/lib/helper.js index 6e77c4ae050..3aab9f523d7 100644 --- a/packages/ember-htmlbars/lib/helper.js +++ b/packages/ember-htmlbars/lib/helper.js @@ -20,6 +20,4 @@ export function helper(helperFn) { }; } -Helper.helper = helper; - export default Helper; diff --git a/packages/ember-htmlbars/lib/main.js b/packages/ember-htmlbars/lib/main.js index c9575b86783..0237ec353db 100644 --- a/packages/ember-htmlbars/lib/main.js +++ b/packages/ember-htmlbars/lib/main.js @@ -31,7 +31,7 @@ import legacyEachWithKeywordHelper from "ember-htmlbars/helpers/-legacy-each-wit import getHelper from "ember-htmlbars/helpers/-get"; import htmlSafeHelper from "ember-htmlbars/helpers/-html-safe"; import DOMHelper from "ember-htmlbars/system/dom-helper"; -import Helper from "ember-htmlbars/helper"; +import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; // importing adds template bootstrapping // initializer to enable embedded templates @@ -72,4 +72,7 @@ Ember.HTMLBars = { DOMHelper }; -Ember.Helper = Helper; +if (Ember.FEATURES.isEnabled('ember-htmlbars-helpers')) { + Helper.helper = makeHelper; + Ember.Helper = Helper; +} diff --git a/packages/ember-htmlbars/lib/system/make_bound_helper.js b/packages/ember-htmlbars/lib/system/make_bound_helper.js index a60fa4a98ba..e686263de8b 100644 --- a/packages/ember-htmlbars/lib/system/make_bound_helper.js +++ b/packages/ember-htmlbars/lib/system/make_bound_helper.js @@ -3,7 +3,7 @@ @submodule ember-htmlbars */ -import Helper from "ember-htmlbars/helper"; +import { helper } from "ember-htmlbars/helper"; /** Create a bound helper. Accepts a function that receives the ordered and hash parameters @@ -49,5 +49,5 @@ import Helper from "ember-htmlbars/helper"; @since 1.10.0 */ export default function makeBoundHelper(fn) { - return Helper.helper(fn); + return helper(fn); } diff --git a/packages/ember-htmlbars/tests/helpers/custom_helper_test.js b/packages/ember-htmlbars/tests/helpers/custom_helper_test.js index b2a060b513e..b05e508ab04 100644 --- a/packages/ember-htmlbars/tests/helpers/custom_helper_test.js +++ b/packages/ember-htmlbars/tests/helpers/custom_helper_test.js @@ -1,5 +1,5 @@ import Component from "ember-views/views/component"; -import Helper from "ember-htmlbars/helper"; +import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; import compile from "ember-template-compiler/system/compile"; import { runAppend, runDestroy } from "ember-runtime/tests/utils"; import Registry from "container/registry"; @@ -24,7 +24,7 @@ QUnit.module('ember-htmlbars: custom app helpers', { }); QUnit.test('dashed shorthand helper is resolved from container', function() { - var HelloWorld = Helper.helper(function() { + var HelloWorld = makeHelper(function() { return 'hello world'; }); registry.register('helper:hello-world', HelloWorld); @@ -87,7 +87,7 @@ QUnit.test('dashed helper can recompute a new value', function() { QUnit.test('dashed shorthand helper is called for param changes', function() { var count = 0; - var HelloWorld = Helper.helper(function() { + var HelloWorld = makeHelper(function() { return ++count; }); registry.register('helper:hello-world', HelloWorld); @@ -137,7 +137,7 @@ QUnit.test('dashed helper compute is called for param changes', function() { QUnit.test('dashed shorthand helper receives params, hash', function() { var params, hash; - var HelloWorld = Helper.helper(function(_params, _hash) { + var HelloWorld = makeHelper(function(_params, _hash) { params = _params; hash = _hash; }); @@ -201,7 +201,7 @@ QUnit.test('dashed helper usable in subexpressions', function() { }); QUnit.test('dashed helper not usable with a block', function() { - var SomeHelper = Helper.helper(function() {}); + var SomeHelper = makeHelper(function() {}); registry.register('helper:some-helper', SomeHelper); component = Component.extend({ container, @@ -338,7 +338,7 @@ QUnit.test('dashed helper used in subexpression is destroyed', function() { this._super(...arguments); } }); - var JoinWords = Helper.helper(function(params) { + var JoinWords = makeHelper(function(params) { return params.join(' '); }); registry.register('helper:dynamic-segment', DynamicSegment); diff --git a/packages/ember-htmlbars/tests/system/lookup-helper_test.js b/packages/ember-htmlbars/tests/system/lookup-helper_test.js index 6e58f51b853..04597c4f2ed 100644 --- a/packages/ember-htmlbars/tests/system/lookup-helper_test.js +++ b/packages/ember-htmlbars/tests/system/lookup-helper_test.js @@ -1,7 +1,7 @@ import lookupHelper, { findHelper } from "ember-htmlbars/system/lookup-helper"; import ComponentLookup from "ember-views/component_lookup"; import Registry from "container/registry"; -import Helper from "ember-htmlbars/helper"; +import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; function generateEnv(helpers, container) { return { @@ -83,7 +83,7 @@ QUnit.test('looks up a shorthand helper in the container', function() { function someName() { called = true; } - view.container._registry.register('helper:some-name', Helper.helper(someName)); + view.container._registry.register('helper:some-name', makeHelper(someName)); var actual = lookupHelper('some-name', view, env); diff --git a/packages/ember/tests/helpers/helper_registration_test.js b/packages/ember/tests/helpers/helper_registration_test.js index 5b54835db5a..8bc4c22c4d8 100644 --- a/packages/ember/tests/helpers/helper_registration_test.js +++ b/packages/ember/tests/helpers/helper_registration_test.js @@ -2,6 +2,7 @@ import "ember"; import EmberHandlebars from "ember-htmlbars/compat"; import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; +import Helper from "ember-htmlbars/helper"; var compile, helpers, makeBoundHelper; compile = EmberHandlebars.compile; @@ -133,7 +134,7 @@ QUnit.test("Helpers can receive injections", function() { serviceCalled = true; } })); - registry.register('helper:full-name', Ember.Helper.extend({ + registry.register('helper:full-name', Helper.extend({ nameBuilder: Ember.inject.service('name-builder'), compute() { this.get('nameBuilder').build(); From c350340e710c829ef4cbc62aff2fbcb8bddfec29 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Sun, 7 Jun 2015 21:49:46 -0400 Subject: [PATCH 4/4] Revert changes to handlebars lookup --- packages/ember-application/lib/system/resolver.js | 7 +------ .../dependency_injection/default_resolver_test.js | 3 +-- packages/ember-htmlbars/lib/system/lookup-helper.js | 11 +++++++++-- .../ember-htmlbars/tests/system/lookup-helper_test.js | 11 +++++++---- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/ember-application/lib/system/resolver.js b/packages/ember-application/lib/system/resolver.js index da8109d81e3..10ce9843160 100644 --- a/packages/ember-application/lib/system/resolver.js +++ b/packages/ember-application/lib/system/resolver.js @@ -15,7 +15,6 @@ import EmberObject from 'ember-runtime/system/object'; import Namespace from 'ember-runtime/system/namespace'; import helpers from 'ember-htmlbars/helpers'; import validateType from 'ember-application/utils/validate-type'; -import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; export var Resolver = EmberObject.extend({ /* @@ -375,11 +374,7 @@ export default EmberObject.extend({ @public */ resolveHelper(parsedName) { - var resolved = this.resolveOther(parsedName) || helpers[parsedName.fullNameWithoutType]; - if (resolved && !resolved.isHelperFactory && !resolved.isHelperInstance && !resolved.isHTMLBars && typeof resolved === 'function') { - resolved = new HandlebarsCompatibleHelper(resolved); - } - return resolved; + return this.resolveOther(parsedName) || helpers[parsedName.fullNameWithoutType]; }, /** Look up the specified object (from parsedName) on the appropriate diff --git a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js index f90ce7d490b..713ebef475c 100644 --- a/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js +++ b/packages/ember-application/tests/system/dependency_injection/default_resolver_test.js @@ -10,7 +10,6 @@ import EmberObject from "ember-runtime/system/object"; import Namespace from "ember-runtime/system/namespace"; import Application from "ember-application/system/application"; import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; -import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; import makeHandlebarsBoundHelper from "ember-htmlbars/compat/make-bound-helper"; import makeViewHelper from "ember-htmlbars/system/make-view-helper"; import makeHTMLBarsBoundHelper from "ember-htmlbars/system/make_bound_helper"; @@ -145,7 +144,7 @@ QUnit.test("the default resolver resolves helpers on the namespace", function() equal(resolvedShorthand, ShorthandHelper, 'resolve fetches the shorthand helper factory'); equal(resolvedComplete, CompleteHelper, 'resolve fetches the complete helper factory'); - ok(resolvedLegacy instanceof HandlebarsCompatibleHelper, 'legacy function helper is wrapped in HandlebarsCompatibleHelper'); + ok(typeof resolvedLegacy === 'function', 'legacy function helper is resolved'); equal(resolvedView, ViewHelper, 'resolves view helper'); equal(resolvedLegacyHTMLBars, LegacyHTMLBarsBoundHelper, 'resolves legacy HTMLBars bound helper'); equal(resolvedLegacyHandlebars, LegacyHandlebarsBoundHelper, 'resolves legacy Handlebars bound helper'); diff --git a/packages/ember-htmlbars/lib/system/lookup-helper.js b/packages/ember-htmlbars/lib/system/lookup-helper.js index dfa897ea58c..0f738c376a4 100644 --- a/packages/ember-htmlbars/lib/system/lookup-helper.js +++ b/packages/ember-htmlbars/lib/system/lookup-helper.js @@ -5,6 +5,7 @@ import Ember from "ember-metal/core"; import Cache from "ember-metal/cache"; +import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; export var CONTAINS_DASH_CACHE = new Cache(1000, function(key) { return key.indexOf('-') !== -1; @@ -14,6 +15,10 @@ export function validateLazyHelperName(helperName, container, keywords) { return container && CONTAINS_DASH_CACHE.get(helperName) && !(helperName in keywords); } +function isLegacyBareHelper(helper) { + return helper && (!helper.isHelperFactory && !helper.isHelperInstance && !helper.isHTMLBars); +} + /** Used to lookup/resolve handlebars helpers. The lookup order is: @@ -36,9 +41,11 @@ export function findHelper(name, view, env) { if (validateLazyHelperName(name, container, env.hooks.keywords)) { var helperName = 'helper:' + name; if (container._registry.has(helperName)) { - var _helper; - Ember.assert(`The factory for "${name}" is not an Ember helper. Please use Ember.Helper.build to wrap helper functions.`, (_helper = container._registry.resolve(helperName)) && _helper && (_helper.isHelperFactory || _helper.isHelperInstance || _helper.isHTMLBars)); helper = container.lookupFactory(helperName); + if (isLegacyBareHelper(helper)) { + Ember.deprecate(`The helper "${name}" is a deprecated bare function helper. Please use Ember.Helper.build to wrap helper functions.`); + helper = new HandlebarsCompatibleHelper(helper); + } } } } diff --git a/packages/ember-htmlbars/tests/system/lookup-helper_test.js b/packages/ember-htmlbars/tests/system/lookup-helper_test.js index 04597c4f2ed..928b2172c0e 100644 --- a/packages/ember-htmlbars/tests/system/lookup-helper_test.js +++ b/packages/ember-htmlbars/tests/system/lookup-helper_test.js @@ -2,6 +2,7 @@ import lookupHelper, { findHelper } from "ember-htmlbars/system/lookup-helper"; import ComponentLookup from "ember-views/component_lookup"; import Registry from "container/registry"; import Helper, { helper as makeHelper } from "ember-htmlbars/helper"; +import HandlebarsCompatibleHelper from "ember-htmlbars/compat/helper"; function generateEnv(helpers, container) { return { @@ -95,7 +96,7 @@ QUnit.test('looks up a shorthand helper in the container', function() { }); QUnit.test('fails with a useful error when resolving a function', function() { - expect(1); + expect(2); var container = generateContainer(); var env = generateEnv(null, container); var view = { @@ -105,7 +106,9 @@ QUnit.test('fails with a useful error when resolving a function', function() { function someName() {} view.container._registry.register('helper:some-name', someName); - expectAssertion(function() { - lookupHelper('some-name', view, env); - }, /The factory for "some-name" is not an Ember helper/); + var actual; + expectDeprecation(function() { + actual = lookupHelper('some-name', view, env); + }, /helper "some-name" is a deprecated bare function helper/); + ok(actual instanceof HandlebarsCompatibleHelper, 'function looks up as compat helper'); });