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

Regression in 0.13: refs of siblings aren't resolved in previous siblings componentDidMount #3128

Closed
sebmarkbage opened this issue Feb 12, 2015 · 4 comments
Assignees

Comments

@sebmarkbage
Copy link
Collaborator

Not sure how to fix this. Probably needs separate queues.

@sophiebits
Copy link
Collaborator

Hmm. We can't just have a refs queue and a componentDidMount queue because if we have

<div>
  <Pa><Ca1 /><Ca2 /></Pa>
  <Pb><Cb1 /><Cb2 /></Pb>
</div>

and we have a ref to each component, then the order should be

Ca1 componentDidMount
Ca2 componentDidMount
Cb1 componentDidMount
Cb2 componentDidMount
Ca1 ref (or these could be interleaved too)
Ca2 ref
Cb1 ref
Cb2 ref
Pa componentDidMount
Pb componentDidMount
Pa ref
Pb ref

I think. Specifically, the child refs need to resolve before the parent's componentDidMount is called. Right now we do

Ca1 componentDidMount
Ca1 ref
Ca2 componentDidMount
Ca2 ref
Pa componentDidMount
Pa ref
Cb1 componentDidMount
Cb1 ref
Cb2 componentDidMount
Cb2 ref
Pb componentDidMount
Pb ref

but you're saying Pa componentDidMount needs to come after Cb{1,2} ref.

@sebmarkbage
Copy link
Collaborator Author

We could fire refs before the child's componentDidMount. That would guarantee that all refs are resolved before any componentDidMounts are fired. I'm not sure what is best or worse.

It seems like we only have two reasonable choices. Either that, or keeping what is in master. I think the choice needs to be based on what is easiest to deal with when you're trying to compose two events.

@sophiebits sophiebits assigned sebmarkbage and unassigned sophiebits Feb 17, 2015
@sophiebits
Copy link
Collaborator

You've thought about this more and I haven't been working on this, so assigning this back to you.

@sebmarkbage
Copy link
Collaborator Author

I think we'll ship an RC in the current state and see what havoc it causes.

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

2 participants