-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Implement Array.prototype.at() #1613
Changes from 10 commits
f8b1997
b2a835f
270508d
7464179
be10c9f
db5725a
07aebb9
0096fd5
138a755
f9bb0ac
57b51ce
1a9c4a9
18e26e3
224dc93
e11bfc4
753f517
1acc870
0a6d4eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -86,6 +86,7 @@ impl BuiltIn for Array { | |||||
values_function, | ||||||
Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::CONFIGURABLE, | ||||||
) | ||||||
.method(Self::at, "at", 1) | ||||||
.method(Self::concat, "concat", 1) | ||||||
.method(Self::push, "push", 1) | ||||||
.method(Self::index_of, "indexOf", 1) | ||||||
|
@@ -487,6 +488,45 @@ impl Array { | |||||
Ok(a.into()) | ||||||
} | ||||||
|
||||||
///'Array.prototype.at(index)' | ||||||
/// | ||||||
/// When at method takes desired integer as index and returns the value at given | ||||||
/// index. Negative integer counts backwards. If index is invalid, the at method | ||||||
/// returns undefined. | ||||||
/// | ||||||
/// More information: | ||||||
/// - [ECMAScript reference][spec] | ||||||
/// - [MDN documentation][mdn] | ||||||
/// | ||||||
/// [spec]: https://tc39.es/ecma262/#sec-array.prototype.at | ||||||
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/at | ||||||
pub(crate) fn at(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult<JsValue> { | ||||||
//1. let O be ? ToObject(this value) | ||||||
let obj = this.to_object(context)?; | ||||||
//2. let len be ? LengthOfArrayLike(O) | ||||||
let len = obj.length_of_array_like(context)? as i64; | ||||||
//3. let relativeIndex be ? ToIntegerOrInfinity(index) | ||||||
let relative_index = args.get(0).unwrap().to_integer_or_infinity(context)?; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This won't panic if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed this literally like 15-20 minutes ago! Completely unaware that get_or_undefined existed! Definitely making this update. |
||||||
let k = match relative_index { | ||||||
//4. if relativeIndex >= 0, then let k be relativeIndex | ||||||
IntegerOrInfinity::Integer(i) if i >= 0 => i, | ||||||
//5. Else, let k be len + relativeIndex | ||||||
IntegerOrInfinity::Integer(i) => len + i, | ||||||
jedel1043 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
//handle most likely impossible case of | ||||||
//IntegerOrInfinity::NegativeInfinity || IntegerOrInfinity::PositiveInfinity | ||||||
//by setting k to len which will return undefined below | ||||||
_ => len, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of relying on the condition below, maybe it would be better if we early returned here:
Suggested change
Makes it easier to identify that only finite numbers are valid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely makes more sense to identify only finite numbers being valid |
||||||
}; | ||||||
//6. if k < 0 or k >= len, | ||||||
if k < 0 || k >= len { | ||||||
//return undefined | ||||||
Ok(JsValue::undefined()) | ||||||
} else { | ||||||
//7. Return ? Get(O, !ToString(𝔽(k))) | ||||||
obj.get(k, context) | ||||||
} | ||||||
} | ||||||
|
||||||
/// `Array.prototype.concat(...arguments)` | ||||||
/// | ||||||
/// When the concat method is called with zero or more arguments, it returns an | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify this comment? You could instead take the explanation of the function on MDN and paste it here, that should be enough to explain what this does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took the MDN description. I thought that one was the best too, but I was a bit hesitant to just grab it.