From 07ea1892c3eea8e341b792d48412708d46c5bda3 Mon Sep 17 00:00:00 2001 From: Marten Schilstra Date: Mon, 14 Dec 2015 13:56:24 +0100 Subject: [PATCH 1/2] [WIP][BUGFIX Beta] Positional param clash on rerender of contextual component helper --- .../lib/utils/extract-positional-params.js | 5 +-- .../tests/helpers/closure_component_test.js | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/ember-htmlbars/lib/utils/extract-positional-params.js b/packages/ember-htmlbars/lib/utils/extract-positional-params.js index 8747920396b..0eb4690c7d6 100644 --- a/packages/ember-htmlbars/lib/utils/extract-positional-params.js +++ b/packages/ember-htmlbars/lib/utils/extract-positional-params.js @@ -1,6 +1,6 @@ import { assert } from 'ember-metal/debug'; import { Stream } from 'ember-metal/streams/stream'; -import { readArray } from 'ember-metal/streams/utils'; +import { isStream, readArray } from 'ember-metal/streams/utils'; export default function extractPositionalParams(renderNode, component, params, attrs) { let positionalParams = component.positionalParams; @@ -25,9 +25,10 @@ function processNamedPositionalParameters(renderNode, positionalParams, params, for (let i = 0; i < limit; i++) { let param = params[i]; + let isActiveStreamParam = isStream(param) && param.isActive; assert(`You cannot specify both a positional param (at position ${i}) and the hash argument \`${positionalParams[i]}\`.`, - !(positionalParams[i] in attrs)); + isActiveStreamParam || !(positionalParams[i] in attrs)); attrs[positionalParams[i]] = param; } diff --git a/packages/ember-htmlbars/tests/helpers/closure_component_test.js b/packages/ember-htmlbars/tests/helpers/closure_component_test.js index a95bc19d564..ee67003eac3 100644 --- a/packages/ember-htmlbars/tests/helpers/closure_component_test.js +++ b/packages/ember-htmlbars/tests/helpers/closure_component_test.js @@ -339,6 +339,38 @@ if (isEnabled('ember-contextual-components')) { }, `You cannot specify both a positional param (at position 0) and the hash argument \`name\`.`); }); + QUnit.test('conflicting positional and hash parameters does not raise and assertion if rerendered', function() { + let LookedUp = Component.extend(); + LookedUp.reopenClass({ + positionalParams: ['name'] + }); + owner.register( + 'component:-looked-up', + LookedUp + ); + owner.register( + 'template:components/-looked-up', + compile(`{{greeting}} {{name}}`) + ); + + let template = compile( + `{{component (component "-looked-up" name greeting="Hodi")}}` + ); + + component = Component.extend({ + [OWNER]: owner, + template, + name: 'Hodari' + }).create(); + + runAppend(component); + equal(component.$().text(), 'Hodi Hodari', 'component is rendered'); + + run(() => component.set('name', 'Sergio')); + + equal(component.$().text(), 'Hodi Sergio', 'component is rendered'); + }); + QUnit.test('conflicting positional and hash parameters does not raise and assertion if in the different closure', function() { let LookedUp = Component.extend(); LookedUp.reopenClass({ From 4e7fe484a09e7f4830cb0d95c7195c037f556956 Mon Sep 17 00:00:00 2001 From: Sergio Arbeo Date: Sun, 20 Dec 2015 14:22:57 +0100 Subject: [PATCH 2/2] [BUGFIX beta] Create a new hash parameter when creating a component cell If we merge positional and hash parameters without duplicating the hash before, then we get an error when rerendering. Fixes test in #12711 --- packages/ember-htmlbars/lib/hooks/component.js | 7 +++++-- packages/ember-htmlbars/lib/keywords/closure-component.js | 7 +++++-- .../ember-htmlbars/lib/utils/extract-positional-params.js | 5 ++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/ember-htmlbars/lib/hooks/component.js b/packages/ember-htmlbars/lib/hooks/component.js index 4dbb42a34ea..b5ae33b6d34 100644 --- a/packages/ember-htmlbars/lib/hooks/component.js +++ b/packages/ember-htmlbars/lib/hooks/component.js @@ -3,6 +3,8 @@ import { assert } from 'ember-metal/debug'; import ComponentNodeManager from 'ember-htmlbars/node-managers/component-node-manager'; import buildComponentTemplate, { buildHTMLTemplate } from 'ember-views/system/build-component-template'; import lookupComponent from 'ember-htmlbars/utils/lookup-component'; +import assign from 'ember-metal/assign'; +import EmptyObject from 'ember-metal/empty_object'; import Cache from 'ember-metal/cache'; import { CONTAINS_DASH_CACHE, @@ -42,9 +44,10 @@ export default function componentHook(renderNode, env, scope, _tagName, params, * on top of the closure component attributes. * */ - processPositionalParamsFromCell(componentCell, params, attrs); + let newAttrs = assign(new EmptyObject(), attrs); + processPositionalParamsFromCell(componentCell, params, newAttrs); params = []; - attrs = mergeInNewHash(componentCell[COMPONENT_HASH], attrs); + attrs = mergeInNewHash(componentCell[COMPONENT_HASH], newAttrs); } } diff --git a/packages/ember-htmlbars/lib/keywords/closure-component.js b/packages/ember-htmlbars/lib/keywords/closure-component.js index 3c53d273435..f8f620fa7d9 100644 --- a/packages/ember-htmlbars/lib/keywords/closure-component.js +++ b/packages/ember-htmlbars/lib/keywords/closure-component.js @@ -7,6 +7,7 @@ import { assert } from 'ember-metal/debug'; import isNone from 'ember-metal/is_none'; import symbol from 'ember-metal/symbol'; import BasicStream from 'ember-metal/streams/stream'; +import EmptyObject from 'ember-metal/empty_object'; import { read } from 'ember-metal/streams/utils'; import { labelForSubexpr } from 'ember-htmlbars/hooks/subexpr'; import assign from 'ember-metal/assign'; @@ -54,12 +55,14 @@ function createClosureComponentCell(env, originalComponentPath, params, hash, la assert(`Component path cannot be null in ${label}`, !isNone(componentPath)); + let newHash = assign(new EmptyObject(), hash); + if (isComponentCell(componentPath)) { - return createNestedClosureComponentCell(componentPath, params, hash); + return createNestedClosureComponentCell(componentPath, params, newHash); } else { assert(`The component helper cannot be used without a valid component name. You used "${componentPath}" via ${label}`, isValidComponentPath(env, componentPath)); - return createNewClosureComponentCell(env, componentPath, params, hash); + return createNewClosureComponentCell(env, componentPath, params, newHash); } } diff --git a/packages/ember-htmlbars/lib/utils/extract-positional-params.js b/packages/ember-htmlbars/lib/utils/extract-positional-params.js index 0eb4690c7d6..8747920396b 100644 --- a/packages/ember-htmlbars/lib/utils/extract-positional-params.js +++ b/packages/ember-htmlbars/lib/utils/extract-positional-params.js @@ -1,6 +1,6 @@ import { assert } from 'ember-metal/debug'; import { Stream } from 'ember-metal/streams/stream'; -import { isStream, readArray } from 'ember-metal/streams/utils'; +import { readArray } from 'ember-metal/streams/utils'; export default function extractPositionalParams(renderNode, component, params, attrs) { let positionalParams = component.positionalParams; @@ -25,10 +25,9 @@ function processNamedPositionalParameters(renderNode, positionalParams, params, for (let i = 0; i < limit; i++) { let param = params[i]; - let isActiveStreamParam = isStream(param) && param.isActive; assert(`You cannot specify both a positional param (at position ${i}) and the hash argument \`${positionalParams[i]}\`.`, - isActiveStreamParam || !(positionalParams[i] in attrs)); + !(positionalParams[i] in attrs)); attrs[positionalParams[i]] = param; }