diff --git a/packages/ember-glimmer/tests/integration/components/closure-components-test.js b/packages/ember-glimmer/tests/integration/components/closure-components-test.js index 2f82fc325c7..43bd7b5cdb7 100644 --- a/packages/ember-glimmer/tests/integration/components/closure-components-test.js +++ b/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({ diff --git a/packages/ember-htmlbars/lib/hooks/component.js b/packages/ember-htmlbars/lib/hooks/component.js index dbe115b40f2..e7e17e80f90 100644 --- a/packages/ember-htmlbars/lib/hooks/component.js +++ b/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); } } diff --git a/packages/ember-htmlbars/lib/keywords/closure-component.js b/packages/ember-htmlbars/lib/keywords/closure-component.js index 4696ce90ced..3dbd5d19e6c 100644 --- a/packages/ember-htmlbars/lib/keywords/closure-component.js +++ b/packages/ember-htmlbars/lib/keywords/closure-component.js @@ -4,6 +4,7 @@ */ 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'; @@ -11,7 +12,7 @@ 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; } diff --git a/packages/ember-htmlbars/lib/keywords/element-component.js b/packages/ember-htmlbars/lib/keywords/element-component.js index adc0debd217..fb51c92e468 100644 --- a/packages/ember-htmlbars/lib/keywords/element-component.js +++ b/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 }; diff --git a/packages/ember-htmlbars/lib/utils/extract-positional-params.js b/packages/ember-htmlbars/lib/utils/extract-positional-params.js index d4e6d617b12..d12545818a4 100644 --- a/packages/ember-htmlbars/lib/utils/extract-positional-params.js +++ b/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);