Fix false "Expected at most 2 arguments" error for variadic min()/max() - #9120
Merged
limzykenneth merged 2 commits intoSep 1, 2026
Merged
Conversation
…/max()
The runtime implementation of min() and max() is variadic, but their
documented overloads only declared (n0, n1), so the parameter validator
rejected calls like min(1, 2, 3, 4). Add a {...Number} rest parameter to
the two-number overload of both functions (same pattern as createVector)
and update parameterData.json accordingly.
Calls with fewer than two number arguments still fail validation with
the same friendly error as before, and the single-array overload is
unchanged.
Member
|
Looks good. Thanks! |
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
2 participants
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.
Resolves #9039
Changes:
The runtime implementation of
min()andmax()is variadic, but their documented overloads only declared(n0, n1), so the FES parameter validator (which derives its Zod schemas from the inline reference viadocs/parameterData.json) rejected calls likemin(1, 2, 3, 4)with a false "Expected at most 2 arguments, but received more in min()". This restores the 1.x behavior (the 1.x version of this same problem was fixed in #4890).@param {...Number} restparameter to the two-number overload of bothmin()andmax()insrc/math/calculation.js, following the existing pattern used bycreateVector().docs/parameterData.jsonso the first overload reads["Number", "Number", "...Number[]"]for both functions. I edited just these two entries by hand: running the fullnpm run docslocally reorders hundreds of unrelated entries (the checked-in file seems out of sync with a full regeneration), which would have buried the actual change. I did verify that a full regeneration produces exactly these values formin/max— happy to commit the fully regenerated file instead if you prefer.test/unit/core/param_errors.jscovering both functions (valid: two numbers, more than two numbers, a single array; invalid: no arguments, a single number, a non-number among the arguments).Following up on @limzykenneth's notes in the issue:
min()andmin(5)still fail validation, and the friendly error message is byte-identical to the current behavior (I compared the old and new schemas side by side).min([1, 2, 3])is unchanged and still valid.param_validator.jswere needed, since it already supports rest parameters (...Number[]) viaz.tuple's rest argument.min(1, 2, '3', 4)→ "Expected number at the third parameter, but received string in min()".Test results:
test/unit/math/+test/unit/core/— 646 passed, 0 failed.PR Checklist
npm run lintpasses