-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
only set a key to VNode have similar shape, fix #5618 #5622
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
- Flow definition on line 39 needs to be updated (the argument is no longer optional)
- We can use a shorter initial index string (like
0
)
9009a76
to
0b06db8
Compare
test still failed, I am try to find the reason. |
You can compile the code, and then copy the test case to the console debugging |
@gebilaoxiong , thanks.
my fault, |
0b06db8
to
1fcb301
Compare
normalizeChildren
, fix #5618
@yyx990803 , my change would introduce new bug :(, because if we force every VNode has a key by order, this will have side effect for patch, see this demo. I have update my change, maybe we should filter the VNode before set a key. |
1fcb301
to
920ddc2
Compare
@@ -45,7 +45,11 @@ function normalizeArrayChildren (children: any, nestedIndex?: string): Array<VNo | |||
last = res[res.length - 1] | |||
// nested | |||
if (Array.isArray(c)) { | |||
res.push.apply(res, normalizeArrayChildren(c, `${nestedIndex || ''}_${i}`)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here only need to determine whether c
is created by slot
, avoid to set key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me fix it 🙂
hi, I looked at your code there is a problem about the nested child as a <div id="app"></div> const vm = new Vue({
el: '#app',
data: { n: 1 },
render(h) {
const list = []
for (let i = 0; i < this.n; i++) {
list.push(h('input', {
attrs: {
value: 'a',
type: 'text'
}
}))
}
const b = h('input', {
attrs: {
value: 'b',
type: 'text'
}
})
return h('div', [
[...list, b]
])
}
}) If the nested list is followed by the same type of element... steps to reproduceEnter some data in the last input box modify the value of vm.n++ What is expectedit will create a new input, and the value of this input is b What is actually happening?But the correct should be create a element In front of ConclusionIt is always problematic to distinguish whether the element is a loop So I think here should only set the key for to avoid other situations |
@gebilaoxiong , you are right, the best way is coding carefully... |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
in issue #5618 , then VNode in the first layer of the array don't get a key when father component call
normalizeChildren
,but get a key when child component call
normalizeChildren
.this leads to a changeable key, and component was re-created.