search

miguelcobain / ember-composability-tools

ember-composability-tools - Helpers for building a somewhat different kind of components.
MIT License
39 stars 18 forks source link

Fix run order of `didInsertParent` for children of already-set-up parents #11

Closed reidab closed 2 years ago

reidab commented 7 years ago

Fixes #10

reidab commented 6 years ago

I'd completely forgotten about this PR until I went to update another library I had that depended on it. Apologies!

I've updated the PR to split out the new tests into their own test case and reverted back the changes to existing tests.

reidab commented 6 years ago

Ping. I've been delaying a library depending on this fix for many months now. Is there anything else I can do to move it forward?

reidab commented 6 years ago

Just rebased the branch on the current master resolved conflicts.

Duder-onomy commented 6 years ago

@miguelcobain I just made PR #19 that will fix the Travis failures seen in this PR.

reidab commented 6 years ago

The description in @Duder-onomy's PR #19 said that it fixed this, so this got auto-closed, but it hasn't actually been merged. 😢

Duder-onomy commented 6 years ago

@miguelcobain This is still an issue, My PR accidentally closed this.. All my PR did was fix TravisCi. Can you re-open this? Or even better, merge it?

Duder-onomy commented 5 years ago

@miguelcobain Can this be merged?

Duder-onomy commented 5 years ago

@miguelcobain This is still an issue, i'm going to try and explain the issue the best I can.

When the change was made to register children on init() instead of didInsertElement(), children of an already-setup parent ( the children were hidden behind some conditional ) are receiving didInsertParent on init, which breaks and code in the didInsertParent hook that relies on the element being available.

Duder-onomy commented 5 years ago

I believe this is the same issue seen in #16 and pull requested in #17.

knownasilya commented 3 years ago

Ping, just for visibility.

miguelcobain commented 2 years ago

The new architecture of ember-composability-tools is a lot different. I added a modified version of this test to the current test suite and it passed. I can't keep the test though, because it required some hacking of the Node component to make the test possible (Good ol' times when we could just willRender=this.spy and it would end up replacing the component's willRender 😅 ).

In any case, I believe this is fixed, according to my tests. Feel free to reopen if this issue is found.