-
Notifications
You must be signed in to change notification settings - Fork 29
Add normative-conventions.md; document agreed-upon coercion rules #136
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
420538b
Add normative-conventions.md; document agreed-upon coercion rules
bakkot 15a3052
fix lint
bakkot 3afdf98
tweak
bakkot b67c068
add integer-taking rule
bakkot 0dc1e70
explicitly note NaN
bakkot 382c1d6
update with outcomes of recent discussions
bakkot e7d60e8
fix dumb lint rule
bakkot f615c1d
typo fix
bakkot d7339d4
mention primitive->object as well
bakkot 89af963
Update normative-conventions.md
bakkot 31ddafb
Update normative-conventions.md
bakkot b750297
reword note about incompleteness
bakkot ffb742b
Merge branch 'main' into add-stop-coercing-things-pt-1
bakkot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Normative conventions | ||
|
|
||
| This document describes some of the conventions that we try to follow when designing new features. Sometimes old features don't follow these; that's ok, new features generally should anyway. | ||
|
|
||
| None of these rules are inviolable, but you should have a good reason for any particular feature to deviate (for example, because the feature is a close cousin of an existing feature, like `findLastIndex` vs `findIndex`). | ||
|
|
||
| This list is very far from being complete. | ||
|
|
||
| ## Number-taking inputs should reject `NaN` | ||
|
|
||
| In any built-in function which expects a non-NaN number (including as an option in an options bag), receiving `NaN` or anything which results in `NaN` after coercion is performed (such as by passing through `ToNumeric`, `ToPrimitive`, `ToNumber`, etc.) should cause a `RangeError` to be thrown. | ||
|
|
||
| Note that some abstract operations, like [ToIntegerOrInfinity](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tointegerorinfinity), [ToIndex](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-toindex), etc, will coerce inputs to Number and then further coerce the result, sometimes including mapping `NaN` to non-`NaN` values. So you'll need to coerce inputs to a Number using `ToNumber` before calling those operations, check for `NaN`, and then pass the resulting Number to the operation. | ||
|
|
||
| An exception to this rule is functions which take optional numeric arguments: in that case, receiving `undefined` should be treated as the argument not being passed. | ||
|
|
||
| NB: This convention is new as of 2023, and most earlier parts of the language do not follow it. | ||
|
bakkot marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Integer-taking inputs should reject non-integral numbers | ||
|
|
||
| In any built-in function which expects an integral number (including as an option in an options bag), receiving a non-integral number or anything which results in a non-integral number after coercion is performed (such as by passing through `ToNumeric`, `ToPrimitive`, `ToNumber`, etc.) should cause a `RangeError` to be thrown. | ||
|
|
||
| Note that some abstract operations, like [ToIntegerOrInfinity](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-tointegerorinfinity), [ToIndex](https://tc39.es/ecma262/multipage/abstract-operations.html#sec-toindex), etc, will coerce inputs to Number and then further round the result. So you'll need to coerce inputs to a Number using `ToNumber` before calling those operations and then check for non-integral values, rather than using those operations. | ||
|
|
||
| For the purposes of this guideline `-0` is considered to be an integral number (specifically, 0). | ||
|
|
||
| Some APIs intentionally round non-integral inputs, for example as an attempt to provide a best-effort behavior, rather than because the API fundamentally only makes sense with integers. This guideline does not apply to those cases. | ||
|
bakkot marked this conversation as resolved.
Outdated
|
||
|
|
||
| NB: This convention is new as of 2023, and most earlier parts of the language do not follow it. | ||
|
|
||
| ## If an argument is expected, not getting it or getting `undefined` should throw a `TypeError` | ||
|
|
||
| When there is a required argument, finding that it is explicitly `undefined` or is missing should throw a `TypeError` instead of attempting to coerce `undefined` to the expected type. This extends to values looked up in options bags, etc. This does not apply when there is a default value for the argument, since in that case it is not required. | ||
|
|
||
| This convention applies only to `undefined` specifically, and says nothing about the appropriate handling of values like `null` and `document.all`. | ||
|
michaelficarra marked this conversation as resolved.
Outdated
|
||
|
|
||
| NB: This convention is new as of 2023, and most earlier parts of the language do not follow it. | ||
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.
Uh oh!
There was an error while loading. Please reload this page.