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

positionalParams causing extra change notifications on initial render #11686

Closed
stefanpenner opened this issue Jul 8, 2015 · 5 comments
Closed
Assignees

Comments

@stefanpenner
Copy link
Member

So today, we must create the component, extract the position args and then place them on attrs. This means we currently always dirty this.attrs even on initial render. At the very best this results in wasteful changeEvents, at the worse may cause a hefty work amplification.

I feel strongly, we need to work hard to ensure attrs on initial render is in its fully formed initial state.

Some ideas:

  • extract positionArgs to a static property so it is readable before creation of the component and can be passed into create.
  • read positionArgs from the prototype so it can be passed to create, this feels icky and doing the same in QP has cause much pain.
  • add positional args at some level during component.init

note: it is true that we also invalidate the keys on this.attrs on initial render, we also need to solve that. But both this and that are tightly related.

@stefanpenner
Copy link
Member Author

@ef4 I've assigned you, to at least provide feedback, since you are likely most familiar with this.

@ef4
Copy link
Contributor

ef4 commented Jul 15, 2015

I concur that the positionalArgs array itself is intended to be a static property of the class, and it should be fair game to access it before instantiation of a component instance.

@stefanpenner
Copy link
Member Author

I will document + assert accordingly, and then take advantage of this concept to being addressing issues the compat code has introduced.

@stefanpenner
Copy link
Member Author

Ok, i think we will treat positionArgs as if it was static, and write some assertions to ensure users don't use it any other way. The final resting place will likely be static, but we can support bothwithout cost today.

This will enable us to begin fixing #11502 (comment)

@rwjblue
Copy link
Member

rwjblue commented Jul 30, 2015

Brain dump after discussing with @stefanpenner in chat:

Go forward would be:

var Thing = Ember.Component.extend({
 // stuff here
});

Thing.reopenClass({
  positionalParams: ['stuff', 'here']
});

export default Thing;

Deprecated but functional:

export default Ember.Component.extend({
  positionalParams: ['stuff', 'here']
});

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

No branches or pull requests

3 participants