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

Ensure Vue instance's vnode and element is up to date #4299

Merged
merged 3 commits into from Nov 23, 2016

Conversation

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Nov 23, 2016

Fix #4284 , to recap the bug, in one sentence:

vm.$el should always be the same as vm.$vnode.elm

More detail:

  1. If a component changes its root element during rendering, all ancestors' placeholder root element's element should be all updated.

  2. If one component is the root element of another component, for example wrapper in the test case, and wrapper's parent is updated (which triggers wrapper's updateFromParent), wrapper's $vnode is not the same as comp's $vnode.parent. While in the initial rendering, these two vnodes are the same one.

To simplify these two statements, we have to keep one invariant: vm.$el should always be the same as vm.$vnode.elm. Otherwise vdom patching will mistakenly insert new dom node.

If we correctly sync vnode's element when child tree's root changes its tag (1), and update vm's vnode when parent is updated(2), we can ensure $el is up to date.

I hope I made myself understood. it is quite perplexing.

I don't think this is the only way to fix the edge case. If @yyx990803 and other members have better alternatives, I'm happy to see it.

@yyx990803 yyx990803 merged commit fce3f04 into vuejs:dev Nov 23, 2016
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@yyx990803
Copy link
Member

yyx990803 commented Nov 28, 2016

@HerringtonDarkholme hey - let me know your email address so I can invite you to the Vue team slack :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.