Merge pull request #13764 from Serabe/fix/13742

[BUGFIX beta] Keep rest positional parameters when nesting contextual components if needed.

packages/ember-glimmer/tests/integration/components/closure-components-test.js

@@ -40,6 +40,142 @@ moduleFor('@htmlbars Components test: closure components', class extends Renderi
     this.assertText('Hodi Hodari');
   }
 
+  ['@test GH#13742 keeps nested rest positional parameters if rendered with no positional parameters']() {
+    this.registerComponent('-looked-up', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{component (component "-looked-up" model.greeting model.name)}}', {
+      model: {
+        greeting: 'Gabon ',
+        name: 'Zack'
+      }
+    });
+
+    this.assertText('Gabon Zack');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('Gabon Zack');
+
+    this.runTask(() => this.context.set('model.greeting', 'Good morning '));
+
+    this.assertText('Good morning Zack');
+
+    this.runTask(() => this.context.set('model.name', 'Matthew'));
+
+    this.assertText('Good morning Matthew');
+
+    this.runTask(() => this.context.set('model',  { greeting: 'Gabon ', name: 'Zack' }));
+
+    this.assertText('Gabon Zack');
+  }
+
+  ['@test overwrites nested rest positional parameters if rendered with positional parameters']() {
+    this.registerComponent('-looked-up', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{component (component "-looked-up" model.greeting model.name) model.name model.greeting}}', {
+      model: {
+        greeting: 'Gabon ',
+        name: 'Zack'
+      }
+    });
+
+    this.assertText('ZackGabon ');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('ZackGabon ');
+
+    this.runTask(() => this.context.set('model.greeting', 'Good morning '));
+
+    this.assertText('ZackGood morning ');
+
+    this.runTask(() => this.context.set('model.name', 'Matthew'));
+
+    this.assertText('MatthewGood morning ');
+
+    this.runTask(() => this.context.set('model',  { greeting: 'Gabon ', name: 'Zack' }));
+
+    this.assertText('ZackGabon ');
+  }
+
+  ['@test GH#13742  keeps nested rest positional parameters if nested and rendered with no positional parameters']() {
+    this.registerComponent('-looked-up', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{component (component (component "-looked-up" model.greeting model.name))}}', {
+      model: {
+        greeting: 'Gabon ',
+        name: 'Zack'
+      }
+    });
+
+    this.assertText('Gabon Zack');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('Gabon Zack');
+
+    this.runTask(() => this.context.set('model.greeting', 'Good morning '));
+
+    this.assertText('Good morning Zack');
+
+    this.runTask(() => this.context.set('model.name', 'Matthew'));
+
+    this.assertText('Good morning Matthew');
+
+    this.runTask(() => this.context.set('model',  { greeting: 'Gabon ', name: 'Zack' }));
+
+    this.assertText('Gabon Zack');
+  }
+
+  ['@test overwrites nested rest positional parameters if nested with new pos params and rendered with no positional parameters']() {
+    this.registerComponent('-looked-up', {
+      ComponentClass: Component.extend().reopenClass({
+        positionalParams: 'params'
+      }),
+      template: '{{#each params as |p|}}{{p}}{{/each}}'
+    });
+
+    this.render('{{component (component (component "-looked-up" model.greeting model.name) model.name model.greeting)}}', {
+      model: {
+        greeting: 'Gabon ',
+        name: 'Zack'
+      }
+    });
+
+    this.assertText('ZackGabon ');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('ZackGabon ');
+
+    this.runTask(() => this.context.set('model.greeting', 'Good morning '));
+
+    this.assertText('ZackGood morning ');
+
+    this.runTask(() => this.context.set('model.name', 'Matthew'));
+
+    this.assertText('MatthewGood morning ');
+
+    this.runTask(() => this.context.set('model',  { greeting: 'Gabon ', name: 'Zack' }));
+
+    this.assertText('ZackGabon ');
+  }
+
   ['@test renders with component helper with curried params, hash']() {
     this.registerComponent('-looked-up', {
       ComponentClass: Component.extend().reopenClass({

packages/ember-htmlbars/lib/hooks/component.js

@@ -8,8 +8,9 @@ import {
 } from 'ember-htmlbars/system/lookup-helper';
 import extractPositionalParams from 'ember-htmlbars/utils/extract-positional-params';
 import {
-  COMPONENT_PATH,
   COMPONENT_HASH,
+  COMPONENT_PATH,
+  COMPONENT_POSITIONAL_PARAMS,
   isComponentCell,
   mergeInNewHash,
   processPositionalParamsFromCell,
@@ -41,8 +42,11 @@ export default function componentHook(renderNode, env, scope, _tagName, params,
        */
       let newAttrs = assign(new EmptyObject(), attrs);
       processPositionalParamsFromCell(componentCell, params, newAttrs);
+      attrs = mergeInNewHash(componentCell[COMPONENT_HASH],
+                             newAttrs,
+                             componentCell[COMPONENT_POSITIONAL_PARAMS],
+                             params);
       params = [];
-      attrs = mergeInNewHash(componentCell[COMPONENT_HASH], newAttrs);
     }
   }
 

packages/ember-htmlbars/lib/keywords/closure-component.js

@@ -4,14 +4,15 @@
 */
 
 import { assert } from 'ember-metal/debug';
+import isEmpty from 'ember-metal/is_empty';
 import isNone from 'ember-metal/is_none';
 import symbol from 'ember-metal/symbol';
 import BasicStream from '../streams/stream';
 import EmptyObject from 'ember-metal/empty_object';
 import { read } from '../streams/utils';
 import { labelForSubexpr } from 'ember-htmlbars/hooks/subexpr';
 import assign from 'ember-metal/assign';
-import { processPositionalParams } from 'ember-htmlbars/utils/extract-positional-params';
+import { isRestPositionalParams, processPositionalParams } from 'ember-htmlbars/utils/extract-positional-params';
 import lookupComponent from 'ember-views/utils/lookup-component';
 
 export const COMPONENT_REFERENCE = symbol('COMPONENT_REFERENCE');
@@ -82,7 +83,10 @@ function createNestedClosureComponentCell(componentCell, params, hash) {
 
   return {
     [COMPONENT_PATH]: componentCell[COMPONENT_PATH],
-    [COMPONENT_HASH]: mergeInNewHash(componentCell[COMPONENT_HASH], hash),
+    [COMPONENT_HASH]: mergeInNewHash(componentCell[COMPONENT_HASH],
+                                     hash,
+                                     componentCell[COMPONENT_POSITIONAL_PARAMS],
+                                     params),
     [COMPONENT_POSITIONAL_PARAMS]: componentCell[COMPONENT_POSITIONAL_PARAMS],
     [COMPONENT_CELL]: true
   };
@@ -124,6 +128,50 @@ function getPositionalParams(container, componentPath) {
   }
 }
 
-export function mergeInNewHash(original, updates) {
-  return assign({}, original, updates);
+/*
+ * This function merges two hashes in a new one.
+ * Furthermore this function deals with the issue expressed in #13742.
+ *
+ * ```hbs
+ * {{component (component 'link-to' 'index')}}
+ * ```
+ *
+ * results in the following error
+ *
+ * > You must provide one or more parameters to the link-to component.
+ *
+ * This is so because a naive merging would not take into account that the
+ * invocation (the external `{{component}}`) would result in the following
+ * attributes (before merging with the ones in the contextual component):
+ *
+ * ```js
+ * let attrs = { params: [] };
+ * ```
+ *
+ * Given that the contextual component has the following attributes:
+ *
+ * ```js
+ * let attrs = { params: ['index'] };
+ * ```
+ *
+ * Merging them would result in:
+ *
+ * ```js
+ * let attrs = { params: [] };
+ * ```
+ *
+ * Therefore, if there are no positional parameters and `positionalParams` is
+ * a string (rest positional parameters), we keep the parameters from the
+ * `original` hash.
+ *
+ */
+export function mergeInNewHash(original, updates, positionalParams=[], params=[]) {
+  let newHash = assign({}, original, updates);
+
+  if (isRestPositionalParams(positionalParams) && isEmpty(params)) {
+    let propName = positionalParams;
+    newHash[propName] = original[propName];
+  }
+
+  return newHash;
 }

packages/ember-htmlbars/lib/keywords/element-component.js

@@ -1,7 +1,8 @@
 import assign from 'ember-metal/assign';
 import {
-  COMPONENT_PATH,
   COMPONENT_HASH,
+  COMPONENT_PATH,
+  COMPONENT_POSITIONAL_PARAMS,
   isComponentCell,
   mergeInNewHash,
   processPositionalParamsFromCell,
@@ -69,8 +70,11 @@ function render(morph, env, scope, [path, ...params], hash, template, inverse, v
 
     // This needs to be done in each nesting level to avoid raising assertions
     processPositionalParamsFromCell(closureComponent, params, hash);
+    hash = mergeInNewHash(closureComponent[COMPONENT_HASH],
+                          hash,
+                          closureComponent[COMPONENT_POSITIONAL_PARAMS],
+                          params);
     params = [];
-    hash = mergeInNewHash(closureComponent[COMPONENT_HASH], hash);
   }
 
   let templates = { default: template, inverse };

packages/ember-htmlbars/lib/utils/extract-positional-params.js

@@ -10,8 +10,12 @@ export default function extractPositionalParams(renderNode, component, params, a
   }
 }
 
+export function isRestPositionalParams(positionalParams) {
+  return typeof positionalParams === 'string';
+}
+
 export function processPositionalParams(renderNode, positionalParams, params, attrs, raiseAssertions = true) {
-  let isRest = typeof positionalParams === 'string';
+  let isRest = isRestPositionalParams(positionalParams);
 
   if (isRest) {
     processRestPositionalParameters(renderNode, positionalParams, params, attrs, raiseAssertions);