limitTo is using isArray

[bathos]

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

The limitTo filter performs a hard isArray check and exits early if this evaluates to false, returning the whole input. Note that the isArray function is Array.isArray, which only returns true for objects which are "precisely" instance of Array, even if they are in fact otherwise instances of Array.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

https://github.com/angular/angular.js/blob/master/src/ng/filter/limitTo.js#L111

What is the expected behavior?

I would expect one of the following:

1. Most natural would be for limitTo to use the same criteria as ngRepeat itself to determine whether its input is an ordered, indexed iterable. NgRepeat uses isArrayLike to make the same judgment and limitTo is a natural companion to ngRepeat. However it is conceivable that not supporting objects which merely have array-like interfaces is intentional, because limitTo uses slice -- not that this couldn't be changed, but it is more involving.
2. That it not ignore Array instances that are not themselves exotic.

What is the motivation / use case for changing the behavior?

As far as I can tell, there's no reason to be restricting limitTo this way, and most likely the isArray usage is an artifact of the time this was first written, because this distinction would not have mattered much a few years ago.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

I came across the problem in 1.4.7, but it appears to be unchanged in the most recent code as well.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

In general, I would suggest Array.isArray is being misused. It's a sort of technical check that returns true only for array exotics, meaning objects created either with literal syntax or the Array constructor, which have exotic proxy traps and which cannot otherwise be identified with total certainty by any other means. Misleading name aside, it is not really for checking if something "is an array" in the regular sense; the way to do this is instanceof Array, as with any other constructor.

I believe issues with using isArray this way are going to become more prevalent now that ES6 makes subclassing Array possible without any special trickery and browser support for this is coming together.

[gkalpak]

Sounds reasonable. As long as the change is backwards compatible and doesn't involve a BC, I would consider it.

Not a high priority, but if anyone feels like submitting a PR and getting the discussion started, please do 😃