Merge pull request #14509 from Serabe/fix/14508

[BUGFIX release] Fix overwriting rest positional parameters when passed as named parameters.
emberjs · Oct 27, 2016 · a27601f · a27601f
2 parents 103784d + d363895
commit a27601f
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 5 deletions.
diff --git a/packages/ember-glimmer/tests/integration/components/closure-components-test.js b/packages/ember-glimmer/tests/integration/components/closure-components-test.js
@@ -3,6 +3,7 @@ import { applyMixins, strip } from '../../utils/abstract-test-case';
 import { moduleFor, RenderingTest } from '../../utils/test-case';
 import assign from 'ember-metal/assign';
 import isEmpty from 'ember-metal/is_empty';
+import { A as emberA } from 'ember-runtime/system/native_array';
 
 moduleFor('@htmlbars Components test: closure components', class extends RenderingTest {
   ['@test renders with component helper']() {
@@ -767,6 +768,83 @@ moduleFor('@htmlbars Components test: closure components', class extends Renderi
     assert.equal(this.$().text(), '');
   }
 
+  ['@test GH#14508 rest positional params are received when passed as named parameter']() {
+    this.registerComponent('my-link', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{component (component "my-link") params=allParams}}', {
+      allParams: emberA(['a', 'b'])
+    });
+
+    this.assertText('ab');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('ab');
+
+    this.runTask(() => this.context.get('allParams').pushObject('c'));
+
+    this.assertText('abc');
+
+    this.runTask(() => this.context.get('allParams').popObject());
+
+    this.assertText('ab');
+
+    this.runTask(() => this.context.get('allParams').clear());
+
+    this.assertText('');
+
+    this.runTask(() => this.context.set('allParams', emberA(['1', '2'])));
+
+    this.assertText('12');
+
+    this.runTask(() => this.context.set('allParams', emberA(['a', 'b'])));
+
+    this.assertText('ab');
+  }
+
+  ['@test GH#14508 rest positional params are received when passed as named parameter with dot notation']() {
+    this.registerComponent('my-link', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{#with (hash link=(component "my-link")) as |c|}}{{c.link params=allParams}}{{/with}}', {
+      allParams: emberA(['a', 'b'])
+    });
+
+    this.assertText('ab');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('ab');
+
+    this.runTask(() => this.context.get('allParams').pushObject('c'));
+
+    this.assertText('abc');
+
+    this.runTask(() => this.context.get('allParams').popObject());
+
+    this.assertText('ab');
+
+    this.runTask(() => this.context.get('allParams').clear());
+
+    this.assertText('');
+
+    this.runTask(() => this.context.set('allParams', emberA(['1', '2'])));
+
+    this.assertText('12');
+
+    this.runTask(() => this.context.set('allParams', emberA(['a', 'b'])));
+
+    this.assertText('ab');
+  }
 });
 
 class ClosureComponentMutableParamsTest extends RenderingTest {

diff --git a/packages/ember-htmlbars/lib/hooks/component.js b/packages/ember-htmlbars/lib/hooks/component.js
@@ -44,6 +44,7 @@ export default function componentHook(renderNode, env, scope, _tagName, params,
       processPositionalParamsFromCell(componentCell, params, newAttrs);
       attrs = mergeInNewHash(componentCell[COMPONENT_HASH],
                              newAttrs,
+                             env,
                              componentCell[COMPONENT_POSITIONAL_PARAMS],
                              params);
       params = [];

diff --git a/packages/ember-htmlbars/lib/hooks/link-render-node.js b/packages/ember-htmlbars/lib/hooks/link-render-node.js
@@ -35,7 +35,7 @@ export default function linkRenderNode(renderNode, env, scope, path, params, has
     let componentCell = stream.value();
 
     if (isComponentCell(componentCell)) {
-      let closureAttrs = mergeInNewHash(componentCell[COMPONENT_HASH], hash);
+      let closureAttrs = mergeInNewHash(componentCell[COMPONENT_HASH], hash, env);
 
       for (let key in closureAttrs) {
         subscribe(renderNode, env, scope, closureAttrs[key]);

diff --git a/packages/ember-htmlbars/lib/keywords/closure-component.js b/packages/ember-htmlbars/lib/keywords/closure-component.js
@@ -60,7 +60,7 @@ function createClosureComponentCell(env, originalComponentPath, params, hash, la
   let newHash = assign(new EmptyObject(), hash);
 
   if (isComponentCell(componentPath)) {
-    return createNestedClosureComponentCell(componentPath, params, newHash);
+    return createNestedClosureComponentCell(componentPath, params, newHash, env);
   } else {
     assert(`The component helper cannot be used without a valid component name. You used "${componentPath}" via ${label}`,
           isValidComponentPath(env, componentPath));
@@ -78,7 +78,7 @@ export function isComponentCell(component) {
   return component && component[COMPONENT_CELL];
 }
 
-function createNestedClosureComponentCell(componentCell, params, hash) {
+function createNestedClosureComponentCell(componentCell, params, hash, env) {
   // This needs to be done in each nesting level to avoid raising assertions.
   processPositionalParamsFromCell(componentCell, params, hash);
 
@@ -87,6 +87,7 @@ function createNestedClosureComponentCell(componentCell, params, hash) {
     [COMPONENT_SOURCE]: componentCell[COMPONENT_SOURCE],
     [COMPONENT_HASH]: mergeInNewHash(componentCell[COMPONENT_HASH],
                                      hash,
+                                     env,
                                      componentCell[COMPONENT_POSITIONAL_PARAMS],
                                      params),
     [COMPONENT_POSITIONAL_PARAMS]: componentCell[COMPONENT_POSITIONAL_PARAMS],
@@ -167,11 +168,14 @@ function getPositionalParams(container, componentPath) {
  * a string (rest positional parameters), we keep the parameters from the
  * `original` hash.
  *
+ * Now we need to consider also the case where the positional params are being
+ * passed as a named parameter.
+ *
  */
-export function mergeInNewHash(original, updates, positionalParams=[], params=[]) {
+export function mergeInNewHash(original, updates, env, positionalParams=[], params=[]) {
   let newHash = assign({}, original, updates);
 
-  if (isRestPositionalParams(positionalParams) && isEmpty(params)) {
+  if (isRestPositionalParams(positionalParams) && isEmpty(params) && isEmpty(env.hooks.getValue(updates[positionalParams]))) {
     let propName = positionalParams;
     newHash[propName] = original[propName];
   }

diff --git a/packages/ember-htmlbars/lib/keywords/element-component.js b/packages/ember-htmlbars/lib/keywords/element-component.js
@@ -73,6 +73,7 @@ function render(morph, env, scope, [path, ...params], hash, template, inverse, v
     processPositionalParamsFromCell(closureComponent, params, hash);
     hash = mergeInNewHash(closureComponent[COMPONENT_HASH],
                           hash,
+                          env,
                           closureComponent[COMPONENT_POSITIONAL_PARAMS],
                           params);
     params = [];