collections breaks Array.from with Iterables

[akloeber]

collections breaks Array.from with Iterables. This can be reproduced with the following code:

var obj = {};
obj[Symbol.iterator] = function() {
  var values = ['foo', 'bar'];

  return {
    next: function() {
      var done = values.length === 0;
      var value = done ? undefined : values.shift();

      return {
        value: value,
        done: done
      };
    }
  };
};

console.log('before collections load:', Array.from(obj));
require('collections/sorted-map');
console.log('after collections load:', Array.from(obj));

Output:

$ node test.js
before collections load: [ 'foo', 'bar' ]
after collections load: []

Environment:
collections v5.0.6
node v4.4.7
Mac OS X 10.12.2

[a-x-]

It implicitly breaks our code too (i had got 4 debug hours) :(

[FrozenPrincess]

Yeah, I lost the better part of a day to this too. It broke some code completely unrelated to anything near the collections stuff, and ended up being a real bitch to track down.

Blindly overwriting Array.from seems like a bad idea, but to do it with a non-conformant implementation really blows my mind.

[marchant]

This Array.from may have been written before the current one even existed or was final. Iterators changed in the past 5 years. So if you're happy with the rest of this library, give the author(s) some credits there. Thanks @a-x- for the PR, it's a step in the right direction to use the native implementation when available, but the shim has to be re-aligned and do what native Array.from does, contributions welcomed!

[kriskowal]

This issue is addressed with Collections version 2. However, this is not backward-compatible with the MontageJS install base, so it is not default. npm install collections@2 to use a modern version that does no monkey patching (unless you make an array instance mutations observable).

[Torniojaws]

The same for Array.prototype.find().

Before:

[5, 12, 8, 130, 44].find((element) => {
  return element > 10;
});
// returns 12

After:

[5, 12, 8, 130, 44].find((element) => {
  return element > 10;
});
// returns -1 
// and also breaks Joi.validate() because of this 

It is a really dangerous game to modify built-in prototypes.

[sergey-shevchuk]

Array.from takes only first parameter into account
Expected syntax: Array.from(arrayLike[, mapFn[, thisArg]])

console.log('before collections load:', Array.from([1, 2, 3], x => x + x););
require('collections/sorted-set');
console.log('after collections load:', Array.from([1, 2, 3], x => x + x););

Output:

$ node test.js
before collections load: [ 2, 4, 6 ]
after collections load: [ 1, 2, 3 ]

[kriskowal]

In my opinion, a major version needs to be released that removes these shims. The simplest solution would be to republish version 2 as a new major that jumps ahead of the latest major versions that are designed to retain backward compatibility with MontageJS.

However, I am no longer vested in this effort. The best people to make this call are @marchant and @hthetiot, who have most recently contributed substantially.

@Stuk and I did have an alternative idea to publish these collections as individual packages under @collections in npm and did a bunch of the footwork, but did not follow through. https://github.com/kriskowal/collections My involvement in JavaScript volunteerism has waned in recent years, since I’m putting more of my time at work in Golang and more of my time at home in text adventure games.

I’m all for enabling any volunteers to bring Collections into the modern era. I imagine this will involve deprecating collections in favor of @collections/* modules and leaving a big note that collections should not be used anymore. This would also involve a pretty substantial update to collectionsjs.com.

[codebling]

See also issues #36 #70 #94 #95 #116 #139 #145 #162 #165 #169 #178 #182 #185 #197 #215 #220 #221 and PRs #94 #95 #116 #173 #189 #212. As mentioned, branch v2 fixes these issues by avoiding global object modification.

[gary-harpaz]

Another victim here. v2 branch solved the issue but I really think a warning should be added to:
https://www.npmjs.com/package/collections
Like in giant red

[aditya1711]

Array.length gets broken for some libraries in React Native, maybe React also, like Victory Native, React native SVG, etc.

I can reproduce the error if required.

Need a fix or workaround for this. Please help.

[codebling]

@aditya1711 have you read the conversation above? Proposed solution is to use v2 branch. Have you tried this?

[aditya1711]

@aditya1711 have you read the conversation above? Proposed solution is to use v2 branch. Have you tried this?

Okay, I didn't know it didn't get merged with the master. It's working now. Thanks a lot for your help and a quick reply.