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

Should min and max be optional? #9

Open
CanadaHonk opened this issue Oct 10, 2024 · 6 comments
Open

Should min and max be optional? #9

CanadaHonk opened this issue Oct 10, 2024 · 6 comments

Comments

@CanadaHonk
Copy link
Owner

Personally unsure currently. Options:

  1. Keep as is, min and max are optional.
  2. Make min (2 args) required, max optional.
  3. Make min and max (3 args) required.

See: https://github.com/tc39/how-we-work/blob/main/normative-conventions.md#when-required-arguments-are-missing-throw

@theScottyJam
Copy link

A few misc thoughts.

Math.clamp(n, 0)

I don't like this. This is basically a shorthand for clamping at a lower-bound, and it just feels wrong for this shorthand to only exist for lower bounds and not upper bounds. I'd rather this just isn't supported, and it gives a TypeError if you tried to do it. I also find it very unintuitive to read code like this.

Math.clamp(42) // => 42

This is just silly. ideally it wouldn't be allowed, but there's not really any harm in allowing it either, since there's no practical reason to ever see this kind of thing.

Math.clamp(n, 0, null);
Math.clamp(n, null, 360);

I find these forms to be useful. You can technically accomplish the same thing with infinity, but I think it's a (tiny) bit more expressive to use a nullish value here instead.

Math.clamp(n, 0, undefined);
Math.clamp(n, undefined, 360);

If using null causes you to fall back to a default parameter, then it makes even more sense for undefined to also do so, as that's how undefined is generally treated in functions.

However, I would find it weird if Math.clamp(n, 0, undefined); were allowed and Math.clamp(n, 0); was not, as most JavaScript functions don't make that kind of distinction.

It also feels really weird to write Math.clamp(n, 0, undefined), as my gut reaction when I see a function like that is to remove the last argument from the argument list, as it isn't doing anything - except doing so would hurt readability in this specific scenario, as discussed above.

So, in the end, it feels like I have this competing set of ideals where I'm going to have to give up something, but I don't know what I would want to give up. Perhaps my preference would be to make all parameters required and non-nullish, and if you want to omit a lower or upper bound, you have to use an infinity - but I don't really know.

@CanadaHonk
Copy link
Owner Author

I am preferring 3 personally and a few other delegates I spoke to hinted towards that option as well; there seems to be a precedent lately to be more strict with argument count. Possibly you could pass undefined/null explicitly but still require 3 arguments, and it could be then default to +-Infinity but undecided on that.

@Richienb
Copy link
Collaborator

Richienb commented Oct 10, 2024

you have to use an infinity

This would make it more cumbersome to use than userland...

It also feels really weird to write Math.clamp(n, 0, undefined), as my gut reaction when I see a function like that is to remove the last argument from the argument list, as it isn't doing anything - except doing so would hurt readability in this specific scenario, as discussed above.

We can still keep them optional, and in this case you would still use Math.clamp(n, 0, Infinity)

@ljharb
Copy link

ljharb commented Oct 10, 2024

fwiw TC39 will no longer be coercing things in new APIs, which means that null and undefined would both throw in this method. undefined would be allowed for an optional argument, of course.

@CanadaHonk
Copy link
Owner Author

Made all required in 513523e

@theScottyJam
Copy link

fwiw TC39 will no longer be coercing things in new APIs, which means that null and undefined would both throw in this method.

If it did coerce, I assume it would coerce to zero, correct?

In my examples, I was assuming that undefined and/or null weren't being coerced, but rather, they had special behavior tied to them to make them behave as shorthand for -Infinity or Infinity.

But I'm good with undefined/null throwing errors and requiring an explicit -Infinity/Infinity instead.

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

No branches or pull requests

4 participants