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

Array properties should be sliced (for concat, destructuring, etc) #595

Closed
bb opened this issue Oct 5, 2016 · 3 comments
Closed

Array properties should be sliced (for concat, destructuring, etc) #595

bb opened this issue Oct 5, 2016 · 3 comments
Assignees
Labels

Comments

@bb
Copy link
Contributor

bb commented Oct 5, 2016

Those 5 example cases should all have the same output, but the 2nd and 4th are currently different:

var regularList = [];
regularList = ["a"].concat(regularList);
regularList = ["b"].concat(regularList);
regularList = ["c"].concat(regularList);
regularList = ["d"].concat(regularList);
console.log(regularList);

var observableList = mobx.observable([]);
observableList = ["a"].concat(observableList);
observableList = ["b"].concat(observableList);
observableList = ["c"].concat(observableList);
observableList = ["d"].concat(observableList);
console.log(observableList);

var myObject = {
  someList: []
 };
myObject.someList = ["a"].concat(myObject.someList);
myObject.someList = ["b"].concat(myObject.someList);
myObject.someList = ["c"].concat(myObject.someList);
myObject.someList = ["d"].concat(myObject.someList);
console.log(myObject.someList);

var myObservableObject = mobx.observable({
  someList: []
 });
myObservableObject.someList = ["a"].concat(myObservableObject.someList);
myObservableObject.someList = ["b"].concat(myObservableObject.someList);
myObservableObject.someList = ["c"].concat(myObservableObject.someList);
myObservableObject.someList = ["d"].concat(myObservableObject.someList);
console.log(myObservableObject.someList);

var observableWithSlice = mobx.observable({
  someList: []
 });
observableWithSlice.someList = ["a"].concat(observableWithSlice.someList.slice());
observableWithSlice.someList = ["b"].concat(observableWithSlice.someList.slice());
observableWithSlice.someList = ["c"].concat(observableWithSlice.someList.slice());
observableWithSlice.someList = ["d"].concat(observableWithSlice.someList.slice());
console.log(observableWithSlice.someList);

Output:

["d","c","b","a"]
["d","c","b","a", []]
["d","c","b","a"]
["d",["c",["b",["a",[]]]]]
["d","c","b","a"]

@mweststrate pointed out on gitter:

@bb that looks like a bug indeed
https://github.com/mobxjs/mobx/blob/master/src/types/observablearray.ts#L264 is the source

EDIT: forgot mobx.observable([]) in initial version

The 2nd example was not using mobx.observable. Sorry for this copy'n'paste error. It's now fixed above. Please note that this brought up another difference for the empty observable array.

@bb
Copy link
Contributor Author

bb commented Oct 5, 2016

After a few more thoughts and debug outputs, it looks to me that an array property of an observable object is not imitating array as good as observableArray does.

Some more differences between the 5 examples:

  • all but the 4th return true for Array.isArray(x) (but all return true for x instanceof Array)
  • browser/nodejs console.log output an array literal for all except the wrong one (which results in type t or ObservableArray {})
  • Object.keys output: ["0","1","2","3"]vs []
  • Object.getOwnPropertyNames output: ["0","1","2","3","length"] vs ["$mobx"]
  • native toString toString.call(x) returns [object Array] vs [object Object]

See https://jsfiddle.net/bock/0bpxtasx/ for executable test cases.

@andykog
Copy link
Member

andykog commented Feb 19, 2017

Don't see any bugs here.

  • 1st example: native arrays works ok
  • 2nd example: nativeArray.concat(observableArray) results in [observableArray] as far as Array.isArray(observableArray) === false
  • 3rd example: concating native array, same as 1st example.
  • 4th example: you get the same result as in 2nd example but after assigning [observableArray] to observable property, it is being automatically converted to ObservableArray so you get ObservableArray["a", observableArray], then ["b" ObservableArray["a", observableArray]] becomes ObservableArray["b", ObservableArray["a", observableArray]] and so on.
  • 5th example: you converting ObservableArray to Array before passing it to concat, so you get native array that, converted to ObservableArray after assining it to observable prop

What's wrong with that?

@mweststrate
Copy link
Member

Thanks for analyzing @andykog!

Behavior in all cases seems to be correct. (Not per se desirable, but alas, proxies.... Safari hurry up!)

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

No branches or pull requests

4 participants