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

call order for didInsertElement for siblings reversed #13972

Closed
KoKuToru opened this issue Jul 30, 2016 · 5 comments
Closed

call order for didInsertElement for siblings reversed #13972

KoKuToru opened this issue Jul 30, 2016 · 5 comments
Assignees
Labels

Comments

@KoKuToru
Copy link

Example:

{{my-component name="A"}}
{{my-component name="B"}}
{{my-component name="C"}}
{{my-component name="D"}}

order of didInsertElement hooks: A B C D

in 2.9.0-alpha+8ba7f68f the order is now: D C B A

Ember-Twiddle: https://ember-twiddle.com/#/847f6adf7c7dc53f65f449062c3c5549 (output in console)

I have some code that depends on the order of didInsertElement.
willInsertElement is still in the right order.

@rwjblue
Copy link
Member

rwjblue commented Jul 30, 2016

Hmm. I don't think the order of didInsertElement for siblings was a really well defined API. We definitely guarantee that any child components' didInsertElement is called before their parents, but I don't think we have made guarantees on the ordering of A, B, C, D in your example.

@krisselden - You likely know this area the best, what do you think? Should we consider this a bug and try to fix it, or is the Glimmer@2 behavior intentional?

@rwjblue rwjblue changed the title call order for didInsertElement reversed call order for didInsertElement for siblings reversed Jul 30, 2016
@chancancode
Copy link
Member

I think we did it to pass tests: 1581583#diff-c8938eb5cb17b733982744e1621cb8e4R168

If the actual goal is to have the children called before the parent, I think the would be pretty simple; instead of queuing the hooks in OpenComponentOpcode, we could move it to CloseComponentOpcode, then we can delete the patch in Ember that reverses the order, and everything should just work.

Are the rules the same for update hooks? I don't know if there is an equivalent place for it now, but we can make CloseComponentOpcode emit a corresponding updating opcode to queue the hook.

@rwjblue
Copy link
Member

rwjblue commented Jul 30, 2016

@chancancode - The parent/child ordering is correct. This issue is about siblings.

@chancancode
Copy link
Member

Yeah. Basically we did pre-order queueing (Parent - Child 1 - Child 2 - Child 3) and then the test failed, so we just reversed the order to make it pass (C3 C2 C1 P).

If we actually want post-order queuing (C1 C2 C3 P), I think we can actually do it. Might have some implications for exception cases though, since if it errored when appending one of your children, your hook will not be queued/called, but that whole area needs a lot more love anyway.

@krisselden
Copy link
Contributor

I don't think it is strictly a bug, but it feels bad.

chancancode added a commit to glimmerjs/glimmer-vm that referenced this issue Sep 7, 2016
chancancode added a commit that referenced this issue Sep 7, 2016
Closes #13972

Requires corresponding Glimmer change
chancancode added a commit to glimmerjs/glimmer-vm that referenced this issue Sep 7, 2016
rwjblue pushed a commit that referenced this issue Sep 7, 2016
Closes #13972

Requires corresponding Glimmer change
webark pushed a commit to webark/ember.js that referenced this issue Oct 6, 2016
Closes emberjs#13972

Requires corresponding Glimmer change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants