Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SassCalculation.clamp with less than 3 arguments fail the validation #2041

Closed
ntkme opened this issue Jul 20, 2023 · 1 comment · Fixed by #2043
Closed

SassCalculation.clamp with less than 3 arguments fail the validation #2041

ntkme opened this issue Jul 20, 2023 · 1 comment · Fixed by #2043
Labels

Comments

@ntkme
Copy link
Contributor

ntkme commented Jul 20, 2023

https://sass-lang.com/documentation/js-api/classes/sasscalculation/#clamp

clamp(min: CalculationValue, value?: CalculationValue, max?: CalculationValue): SassCalculation

In JS API, we can construct clamp with only 1 one argument.

In Dart code documentation:

  /// This may be passed fewer than three arguments, but only if one of the
  /// arguments is an unquoted `var()` string.

Then, this spec seems wrong to throw an error for requiring 3 arguments: https://github.com/sass/sass-spec/blob/e6e6762153c8c6a02d289ab078fb1186c10b91f3/js-api-spec/value/calculation.test.ts#L320-L327

Finally in dart-sass and embedded-host-node, there are extra logic in protofy and deprotofy to enforce 3 arguments:

} else if (calculation.name == "clamp") {
if (calculation.arguments.length != 3) {
throw paramsError(
"Value.Calculation.arguments must have exactly 3 arguments for "
"clamp().");
}
return SassCalculation.clamp(
_deprotofyCalculationValue(calculation.arguments[0]),
_deprotofyCalculationValue(calculation.arguments[1]),
_deprotofyCalculationValue(calculation.arguments[2]));

https://github.com/sass/embedded-host-node/blob/2b1f4ecaf83a9e949976427f522291f674a04a57/lib/src/protofier.ts#L383-L387

These currently makes it impossible to return a clamp with an unquoted var() string in custom functions.

@nex3
Copy link
Contributor

nex3 commented Jul 20, 2023

The _verifyLength() call is correct—that function does include logic to add exceptions for var() calls. The verification logic in the protofier and the JS API tests are indeed wrong, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants