Positional rest params don't work as expected for nested closure components #13742
Comments
|
@Serabe can you take a look here? |
|
Assigning this to myself. Thanks for mentioning, @mixonic! |
|
I think the problem is caused by the merging. Looking for a nice way to fix it. |
Serabe
added a commit
to Serabe/ember.js
that referenced
this issue
Jul 21, 2016
components if needed.
Steps to reproduce the bug before this patch:
1. Create a component with rest positional param (like `link-to`).
2. Create contextual component (`(component "link-to" "index")`).
3. Render it (`{{component (component "link-to" "index")}}`).
4. Realize the component did not receive `"index"` at all.
What was causing it?
====================
Because of how positional parameters work (both in components and contextual
components), it is needed to treat them at each level and then merge them.
Sadly, this was causing to overwrite the rest positional parameters because
when no parameters are passed, the attribute get an empty array nonetheless.
In the example above, assuming `LinkToComponent.positionalParams === 'params'`:
1. Contextual component in `2` gets its positional parameters processed and
receives the following named parameters `{ params: ['index'] }`.
2. When rendering, the `{{component}}` processes its own parameters getting
`{ params: [] }`. Then it merges in a `new EmptyObject()` these attributes
on top of the ones in the contextual component. (See what I did there?
I used the JS keyword `new` as an adjective! He he. Don't judge me, it is
pretty late in here)
Solving the issue
=================
This PR changes the implementation of `mergeInNewHash` to keep the original
positional parameters if the new ones are empty and the positional parameter is
of type rest.
Fixes emberjs#13742
rwjblue
pushed a commit
that referenced
this issue
Jul 25, 2016
… components if needed.
Steps to reproduce the bug before this patch:
1. Create a component with rest positional param (like `link-to`).
2. Create contextual component (`(component "link-to" "index")`).
3. Render it (`{{component (component "link-to" "index")}}`).
4. Realize the component did not receive `"index"` at all.
What was causing it?
====================
Because of how positional parameters work (both in components and contextual
components), it is needed to treat them at each level and then merge them.
Sadly, this was causing to overwrite the rest positional parameters because
when no parameters are passed, the attribute get an empty array nonetheless.
In the example above, assuming `LinkToComponent.positionalParams === 'params'`:
1. Contextual component in `2` gets its positional parameters processed and
receives the following named parameters `{ params: ['index'] }`.
2. When rendering, the `{{component}}` processes its own parameters getting
`{ params: [] }`. Then it merges in a `new EmptyObject()` these attributes
on top of the ones in the contextual component. (See what I did there?
I used the JS keyword `new` as an adjective! He he. Don't judge me, it is
pretty late in here)
Solving the issue
=================
This PR changes the implementation of `mergeInNewHash` to keep the original
positional parameters if the new ones are empty and the positional parameter is
of type rest.
Fixes #13742
(cherry picked from commit 7737eb4)
toddjordan
pushed a commit
to toddjordan/ember.js
that referenced
this issue
Sep 9, 2016
components if needed.
Steps to reproduce the bug before this patch:
1. Create a component with rest positional param (like `link-to`).
2. Create contextual component (`(component "link-to" "index")`).
3. Render it (`{{component (component "link-to" "index")}}`).
4. Realize the component did not receive `"index"` at all.
What was causing it?
====================
Because of how positional parameters work (both in components and contextual
components), it is needed to treat them at each level and then merge them.
Sadly, this was causing to overwrite the rest positional parameters because
when no parameters are passed, the attribute get an empty array nonetheless.
In the example above, assuming `LinkToComponent.positionalParams === 'params'`:
1. Contextual component in `2` gets its positional parameters processed and
receives the following named parameters `{ params: ['index'] }`.
2. When rendering, the `{{component}}` processes its own parameters getting
`{ params: [] }`. Then it merges in a `new EmptyObject()` these attributes
on top of the ones in the contextual component. (See what I did there?
I used the JS keyword `new` as an adjective! He he. Don't judge me, it is
pretty late in here)
Solving the issue
=================
This PR changes the implementation of `mergeInNewHash` to keep the original
positional parameters if the new ones are empty and the positional parameter is
of type rest.
Fixes emberjs#13742
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Consider the following example:
This results in an error:
If you re-write the declaration so that the outer component passes in positional args, then it works:
Which differs in behavior from using curried named parameters. I think the issue is here: https://github.com/emberjs/ember.js/blob/master/packages/ember-htmlbars/lib/utils/extract-positional-params.js#L40
I think that what we really want is to bail out early if
params.lengthis 0, otherwise the condition could be false, which will result in creating a new stream that ultimately returns an empty array. Later the components see this empty array as "no parameters" and they don't work.The text was updated successfully, but these errors were encountered: