A bug when vdom trying to update children when data being changed in some case

[maple-leaf]

Version

2.5.9

Reproduction link

https://codepen.io/maple-leaf/pen/BmgaqG

Steps to reproduce

1. open the codepen demo
2. open devtool if u are on chrome
3. click change data which will raise error button
4. now you should see an error like this: TypeError: Cannot read property 'key' of undefined.
5. refresh demo page
6. click safe to change data here button
7. now you should not see an error like before, and data being updated correctly

What is expected?

maybe same behavior like both should raise an error, but if raise error, maybe duplicate key error should be showed too which will help developer easier to find what data is killing page.

What is actually happening?

both data have duplicate keys, but behavior is different

---

why error being raise when you click change data which will raise error button?

Because the data will raise error has some special changes. It remove all keys except duplicate key 'b' compared to initial data and the order is different.

This will make updateChildren function in vdom/patch.js hit this code section, which will create a oldKeyToIdx. And then when first key 'b' comes, it will set value of b in oldKeyToIdx to undefined, and then next key b comes, it will compare old node and new node via function sameNode. But this time, index of old node which key is b is undefined now. So a js error is being raised.

[posva]

The first button should indeed warn the user of duplicate keys as the second button does (if you change the vue file the a non-minified version)

[maple-leaf]

I think not just not warning being thrown when click on first button should be fixed. The second one which throw a js error and make page unusable on production env, what is more important, should be fixed.

Even thought repeated key is not correctly in common, but this js error is introduced by framework, which will not happen when write valiant script.

[posva]

The error here is expected as such code should never be deployed, that's what dev warnings/erros are there for, to help you prevent that 😄

[maple-leaf]

I think that is good for most case. But in this case, :key will only fire error on some special data which can be hard to be exposed at dev sometimes. And this can be a hidden bug which will crash app at some day.

[gebilaoxiong]

I'm sorry, some changes on the job recently, so follow up late

You may feel very strange

Why changeData didn't trigger warnning

That's because of the new array length is longer than the old

So the last one(duplicate key) need to perform a insert operation

  function addVnodes (parentElm, refElm, vnodes, startIdx, endIdx, insertedVnodeQueue) {
    for (; startIdx <= endIdx; ++startIdx) {
      createElm(vnodes[startIdx], insertedVnodeQueue, parentElm, refElm)
    }
  }

But this operation did not check key

So... there is no warning message

Other case

If a duplicate key is included in data, no warning will be triggered

data: function() {
  return {
    data: [{
      key: 'a',
      value: 'first'
    }, {
      key: 'b',
      value: 'second'
    }, {
      key: 'c',
      value: 'third'
    }, {
      key: 'b',
      value: 'second'
    }]
  }
}

So I think it needs to be checked in these two cases

As to why it is checked only in the development mode

I agree with posva, There are some other reasons for performance

Even in the production environment, I would like to have a good way of doing it