[Fix] parse: fix error message to reflect arrayLimit as max index; remove extraneous comments#545
Merged
ljharb merged 1 commit intoljharb:mainfrom Feb 11, 2026
Merged
Conversation
…remove extraneous comments
The arrayLimit option is documented as limiting the 'maximum index'
in an array, but was incorrectly being applied to the array length.
This caused inconsistent behavior between indexed (param[0]=val) and
non-indexed (param[]=val) array parameters.
With arrayLimit: 3:
- BEFORE: 4 elements (indices 0-3) would convert to object since
length 4 > limit 3
- AFTER: 4 elements (indices 0-3) stays as array since max index
3 <= limit 3
Changes:
- utils.js combine(): Check 'length - 1 > limit' instead of 'length > limit'
- utils.js merge(): Apply arrayLimit when appending to arrays
- parse.js parseArrayValue(): Use '>' instead of '>=' for throwOnLimitExceeded check
Tests updated to reflect correct behavior per documentation.
Fixes ljharb#540.
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Kai Gritun <kai@kaigritun.com>
8b8fc99 to
0ba2bfd
Compare
parse: fix error message to reflect arrayLimit as max index; remove extraneous comments
ljharb
approved these changes
Feb 10, 2026
Owner
ljharb
left a comment
There was a problem hiding this comment.
I reduced this to a more minimal implementation.
6c2db2d to
fbc5206
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #545 +/- ##
=======================================
Coverage 99.75% 99.75%
=======================================
Files 9 9
Lines 2048 2048
Branches 229 229
=======================================
Hits 2043 2043
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced Feb 11, 2026
When
arrayLimit is exceeded and mixed array notation is used, result depends on the limit value
#546
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #540
The
arrayLimitoption is documented as limiting the maximum index in an array:However, it was incorrectly being applied to array length, causing inconsistent behavior between indexed (
param[0]=val) and non-indexed (param[]=val) array parameters.Example (from issue #540)
Changes
combine(): Checklength - 1 > limitinstead oflength > limitmerge(): Apply arrayLimit when appending to arraysparseArrayValue(): Use>instead of>=for throwOnLimitExceeded checkTest Updates
Updated test expectations to reflect correct behavior per documentation:
arrayLimit: 3, arrays with max index 3 (4 elements) now stay as arraysarrayLimit: 0, arrays with max index 0 (1 element) now stay as arrays