Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Positional param 'value' leaks inner data when used in combination with nested component helpers #12717

Closed
ghost opened this issue Dec 16, 2015 · 6 comments · Fixed by #12746
Closed

Comments

@ghost
Copy link

ghost commented Dec 16, 2015

If I have component with the positional param value, and I use it in combination with nested component helpers {{component (component my-component)}}, and the following component code:

let MyComponent = Component.extend({
  didReceiveAttrs() {
    let value = this.getAttr('value');
    console.log(value);
  }
});

MyComponent.reopenClass({ positionalParams: ['value'] });

Then the logged value will be an object with attributes COMPONENT_CELL, COMPONENT_HASH, COMPONENT_PATH and COMPONENT_POSITIONAL_PARAMS.

This fails for me using 2.3.0-beta.2 and components/ember#canary. I'll try to add a failing test if I get to it.
I also believe @Serabe will be delighted to hear about this.

@Serabe
Copy link
Member

Serabe commented Dec 16, 2015

I cannot reproduce it:

https://ember-twiddle.com/bc25554d2cbe7c0c5ced
https://ember-twiddle.com/8991cb8cfb659cd6ea60

Can you please provide more info?

If I remove the quotes around my-component I get an expected error: component path cannot be null.
{{component (component (component "my-component"))}} simulates my-component being a closure itself in your code. I get two undefineds. Can you try to modify the twiddles to show the failure?

Thank you for reporting this!

@ghost
Copy link
Author

ghost commented Dec 16, 2015

@Serabe It seems that the contextual helper is needed to trigger the bug. https://ember-twiddle.com/1eba14b986610d4dc631

@Serabe
Copy link
Member

Serabe commented Dec 16, 2015

Thank you! I'll be fixing this today "hopefully"

@ghost
Copy link
Author

ghost commented Dec 16, 2015

👍

@Serabe
Copy link
Member

Serabe commented Dec 17, 2015

I need @mixonic or @rwjblue guidance on this.

The problem is that this is being rendered as {{c.my-component}} (the fact that is has no params nor attrs is actually really important.

When htmlbars finds {{c.my-component}} it thinks that it is a render, not an inline (which would happen if the call was {{c.my-component 1}}). The problem arises because range sends to handleRedirect [value] as the parameters (can be seen here. I tried removing the value inside the array and all tests pass.

Should I submit a PR for this or is there any reason for that code being there? How can I test ember with a custom htmlbars?

@rwjblue
Copy link
Member

rwjblue commented Dec 22, 2015

Reopening to get the updated version here.

@Serabe - Can you submit a PR updating to HTMLBars v0.14.11?

Serabe added a commit to Serabe/ember.js that referenced this issue Dec 22, 2015
This fixes a bug where a component's cell was being leaked as the first
positional parameter.

Fixes emberjs#12717
rwjblue pushed a commit that referenced this issue Dec 27, 2015
This fixes a bug where a component's cell was being leaked as the first
positional parameter.

Fixes #12717

(cherry picked from commit 697e9e5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants