sindresorhus / is

Type check values
MIT License
1.68k stars 109 forks source link

is.all(is.array, arr1, arr2) tries to use index 1 as a function #123

Closed GuillaumeRochat closed 4 years ago

GuillaumeRochat commented 4 years ago

With the is.array changes to include an additional type check, the is.array function can no longer be used within a is.all. The behavior can also be misleading at first because it works when there is only one element being checked. It may be difficult to debug when used with the spread operator.

E.g.:

> const test1 = [['a']]
> is.all(is.array, ...test1)
true
> const test2 = [['a'], ['b']]
> is.all(is.array, ...test2);
Uncaught TypeError: 1 is not a function
    at Array.every (<anonymous>)
    at is.array (/home/user/project/node_modules/@sindresorhus/is/dist/index.js:137:18)
    at Array.every (<anonymous>)
    at predicateOnArray (/home/user/project/node_modules/@sindresorhus/is/dist/index.js:278:19)
    at Function.is.all (/home/user/project/node_modules/@sindresorhus/is/dist/index.js:284:36)

I can easily change the syntax to

is.all(v => is.array(v), ...test2)

and have it work like it used to, with the added benefit of using it with the new type check when needed

is.all(v => is.array(v, is.string), ...test2)

However, I think the array method could check for if (is.function_(assertion)) instead of just if (assertion), this way it would not try to call the index number as a function (which Array.prototype.every sets as the second argument on its callback).

sindresorhus commented 4 years ago

// @Arnovsky

gioragutt commented 4 years ago

@GuillaumeRochat

However, I think the array method could check for if (is.function_(assertion)) instead of just if (assertion)

This would break if you have an array of functions.

GuillaumeRochat commented 4 years ago

@gioragutt no, it would only check for the assertion callback, making sure it is one, unless there's something else I didn't understand.