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

mergeData cannot works properly with Array #4191

Closed
zhu opened this issue Nov 14, 2016 · 5 comments
Closed

mergeData cannot works properly with Array #4191

zhu opened this issue Nov 14, 2016 · 5 comments

Comments

@zhu
Copy link

zhu commented Nov 14, 2016

Vue.js version

2.0.5

Reproduction Link

https://jsfiddle.net/crjL62kL/4/

Steps to reproduce

1 Create an array
2 Add something to Array.prototype or add some property to the array obj
3 use this array as data and trigger mergeData

What is Expected?

the array is merged withou error and without key-values in prototype

What is actually happening?

TypeError or key-values in prototype is copy to the array

@Jinjiang
Copy link
Member

It seems for (key in from) { ... } in mergeData let them go through.
https://github.com/vuejs/vue/blob/v2.0.5/src/core/util/options.js#L44

@defcc defcc added the 2.0 label Nov 14, 2016
@zhu
Copy link
Author

zhu commented Nov 14, 2016

obj.length = Math.max(obj.length, key) in set throw the TypeError when the array has own properties. I think it's better to merge all elems and properties from the from array.
https://github.com/vuejs/vue/blob/v2.0.5/src/core/observer/index.js#L189

@yyx990803
Copy link
Member

This seems to be a bug in the VueCharts library, because mergeData should never be called on arrays. This means the library is somehow passing an array as the data option, which is not supported.

@zhu
Copy link
Author

zhu commented Nov 15, 2016

The VueCharts library does not pass an array as the data.
https://github.com/vuejs/vue/blob/v2.0.5/src/core/util/options.js#L49
The mergeData will be called recursively. So the function will be called on arrays when the data include array properties.

Here's another example without VueCharts:
https://jsfiddle.net/bdpkrnus/

And I think the mergeData will copy properties from from object's prototype to the to object. Is this by design?

@yyx990803 yyx990803 reopened this Nov 15, 2016
@yyx990803
Copy link
Member

There is indeed a bug in mergeData, but your usage is incorrect: you should not be declaring a prop and a data property using the same key.

After the fix you should see a warning message telling you not to do that.

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

4 participants