Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat(limitTo): ignore limit when undefined #10510

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions src/ng/filter/limitTo.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
* @param {string|number} limit The length of the returned array or string. If the `limit` number
* is positive, `limit` number of items from the beginning of the source array/string are copied.
* If the number is negative, `limit` number of items from the end of the source array/string
* are copied. The `limit` will be trimmed if it exceeds `array.length`
* are copied. The `limit` will be trimmed if it exceeds `array.length`. If `limit` is undefined,
* the input will be returned unchanged.
* @returns {Array|string} A new sub-array or substring of length `limit` or less if input array
* had less than `limit` elements.
*
Expand Down Expand Up @@ -97,13 +98,10 @@ function limitToFilter() {
limit = int(limit);
}

if (isNaN(limit)) return input;

if (isString(input)) {
//NaN check on limit
if (limit) {
return limit >= 0 ? input.slice(0, limit) : input.slice(limit, input.length);
} else {
return "";
}
return limit >= 0 ? input.slice(0, limit) : input.slice(limit, input.length);
}

var i, n;
Expand All @@ -118,7 +116,7 @@ function limitToFilter() {
i = 0;
n = limit;
} else {
// zero and NaN check on limit - return empty array
// zero check on limit - return empty array
if (!limit) return [];

i = input.length + limit;
Expand Down
34 changes: 22 additions & 12 deletions test/ng/filter/limitToSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,30 @@ describe('Filter: limitTo', function() {
});


it('should return an empty array when X cannot be parsed', function() {
expect(limitTo(items, 'bogus')).toEqual([]);
expect(limitTo(items, 'null')).toEqual([]);
expect(limitTo(items, 'undefined')).toEqual([]);
expect(limitTo(items, null)).toEqual([]);
expect(limitTo(items, undefined)).toEqual([]);
it('should return an empty array when X = 0', function() {
expect(limitTo(items, 0)).toEqual([]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit picking: Could add a test for '0' (string) as well.

expect(limitTo(items, '0')).toEqual([]);
});

it('should return an empty string when X cannot be parsed', function() {
expect(limitTo(str, 'bogus')).toEqual("");
expect(limitTo(str, 'null')).toEqual("");
expect(limitTo(str, 'undefined')).toEqual("");
expect(limitTo(str, null)).toEqual("");
expect(limitTo(str, undefined)).toEqual("");
it('should return entire array when X cannot be parsed', function() {
expect(limitTo(items, 'bogus')).toEqual(items);
expect(limitTo(items, 'null')).toEqual(items);
expect(limitTo(items, 'undefined')).toEqual(items);
expect(limitTo(items, null)).toEqual(items);
expect(limitTo(items, undefined)).toEqual(items);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be a test with 0 (to ensure it works as expected).

UPDATE: Has been added.

});

it('should return an empty string when X = 0', function() {
expect(limitTo(str, 0)).toEqual("");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit picking: Could add a test for '0' (string) as well.
(I know it was like this before, but single-quotes would also be preferable to double-quotes for consistency.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

expect(limitTo(str, '0')).toEqual("");
});

it('should return entire string when X cannot be parsed', function() {
expect(limitTo(str, 'bogus')).toEqual(str);
expect(limitTo(str, 'null')).toEqual(str);
expect(limitTo(str, 'undefined')).toEqual(str);
expect(limitTo(str, null)).toEqual(str);
expect(limitTo(str, undefined)).toEqual(str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be a test with 0 (to ensure it works as expected).

UPDATE: Has been added.

});


Expand Down