search

rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.36k stars 194 forks source link

Use `Array.isArray` instead of `instanceof Array` #242

Closed rickharris closed 6 years ago

rickharris commented 6 years ago

:wave: We've been using this library for a while at goabstract.com and I just hit a snag while upgrading from 6.3.0 to 7.1.2. We run our tests with karma and karma-electron-launcher, which uses an iframe to run tests. When I upgraded to 7.1.2, I started getting all sorts of errors from inside of seamless-immutable, saying The first argument to Immutable#setIn must be an array containing at least one "key" string even though I was definitely passing a non-empty array of strings as the first argument to that function. After a while, I realized that, inside of seamless-immutable, path instanceof Array was returning false.

It turns out that MDN recommends Array.isArray over instanceof Array, saying

When checking for Array instance, Array.isArray is preferred over instanceof because it works through iframes.

I've confirmed that this change fixes the iframe issue and that Array.isArray is compatible with the browsers that seamless-immutable tests against. If compatibility is a concern, though, I could add a very small polyfill to this PR.

Thanks @rtfeldman!

qlqllu commented 6 years ago

I have the same issue, when the PR can be merged?

rtfeldman commented 6 years ago

Thanks @rickharris! Published as 7.1.3