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

Remove isarray dependency #212

Closed
jimmywarting opened this issue May 19, 2016 · 8 comments
Closed

Remove isarray dependency #212

jimmywarting opened this issue May 19, 2016 · 8 comments

Comments

@jimmywarting
Copy link
Contributor

think we can replace it with Array.isArray

@calvinmetcalf
Copy link
Contributor

Not on all targeted platforms I don't believe

@mcollina
Copy link
Member

Is Array.isArray present everywhere on all our target platforms?

@jimmywarting
Copy link
Contributor Author

jimmywarting commented May 19, 2016

Wich platform do we target?

@calvinmetcalf
Copy link
Contributor

actually now that I look at it, isArray seems to be available on all the platforms, we can test

@dcousineau
Copy link

The isarray package in its entirety is:

var toString = {}.toString;

module.exports = Array.isArray || function (arr) {
  return toString.call(arr) == '[object Array]';
};

And can more simply be inlined, not required.

@Croydon
Copy link

Croydon commented Jan 9, 2018

Before I have opened #318 I should had searched first. Shame on me :)

So what's the plan here? You discussed in the pull request which was going to remove the isArray dependency, then you decided to make it inline 69d13e3#diff-279988bef5e6fbf7ed82cae28ecc2d59 just to revert this again in the next commit ea4eaba#diff-279988bef5e6fbf7ed82cae28ecc2d59

So...what is the plan?

@jimmywarting
Copy link
Contributor Author

jimmywarting commented Apr 27, 2018

Bumping my thread 2 years later after still seeing it being used in my dependency graph from latest karma (2.0.2)...

Can we bump the major version soon and remove the dependency and perhaps the inlined version also?

looks pretty green to me:
skarmavbild 2018-04-27 kl 20 03 32

Still wonder what platform you are targeting in current version and next major bump

@mcollina
Copy link
Member

Yes, we will do that in the coming weeks. We will cut a new major release from Node 10 as soon as possible (but it will take a while).

mcollina added a commit that referenced this issue Aug 10, 2018
Fixes #283
Fixes #284
Fixes #212
Fixes #297
Fixes #346
Fixes #339
Fixed #335
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

5 participants