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

Tracking issue for implemented methods on JsTypedArray #3252

Open
31 of 39 tasks
jedel1043 opened this issue Sep 1, 2023 · 9 comments
Open
31 of 39 tasks

Tracking issue for implemented methods on JsTypedArray #3252

jedel1043 opened this issue Sep 1, 2023 · 9 comments
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@jedel1043
Copy link
Member

jedel1043 commented Sep 1, 2023

Implementation in https://github.com/boa-dev/boa/blob/main/boa_engine/src/object/builtins/jstypedarray.rs

  • at
  • get buffer
  • get byteLength
  • get byteOffset
  • constructor
  • copyWithin
  • entries
  • every
  • fill
  • filter
  • find
  • findIndex
  • findLast
  • findLastIndex
  • forEach
  • includes
  • indexOf
  • join
  • keys
  • lastIndexOf
  • get length
  • map
  • reduce
  • reduceRight
  • reverse
  • set
  • slice
  • some
  • sort
  • subarray
  • toLocaleString
  • toReversed
  • toSorted
  • toString
  • values
  • with
  • @@iterator
  • get @@toStringTag

Desired features

  • Easily return the inner data as &[u8] or &Vec<u8>
@jedel1043 jedel1043 added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers builtins PRs and Issues related to builtins/intrinsics labels Sep 1, 2023
@AryaveerSR
Copy link

If I understand correctly, the methods need to call the respective method on TypedArray with a reference to the inner value and the arguments, do minor wrapping/unwrapping/conversions etc. and return as a JsValue right ?

@jedel1043
Copy link
Member Author

If I understand correctly, the methods need to call the respective method on TypedArray with a reference to the inner value and the arguments, do minor wrapping/unwrapping/conversions etc. and return as a JsValue right ?

Yes, but some methods like toSorted haven't been implemented for TypedArray yet

@AngeloChecked
Copy link
Contributor

AngeloChecked commented Oct 8, 2023

Hi @jedel1043 and @AryaveerSR
Are you actively addressing this issue? I am willing to assist if there are no concerns about potential overlap?

Just to get better the issue, are these implementations okay, or am I missing something?

/// Calls `TypedArray.prototype.keys()`.
pub fn keys(&self, context: &mut Context<'_>) -> JsResult<JsValue> {
    TypedArray::keys(&self.inner, &[], context)
}

/// Calls `TypedArray.prototype.entries()`.
pub fn entries(&self, context: &mut Context<'_>) -> JsResult<JsValue> {
    TypedArray::entries(&self.inner, &[], context)
}

/// Calls `TypedArray.prototype.findLast()`.
pub fn find_last(&self, predicate: JsFunction, context: &mut Context<'_>) -> JsResult<JsValue> {
    TypedArray::find_last(&self.inner, &[predicate.into()], context)
}

@jedel1043
Copy link
Member Author

@AngeloChecked Yes, those implementations are good! It would be cool to see findLast return Option<JsValue>, but I don't think find does that, so we can leave that as a future improvement.

@jedel1043
Copy link
Member Author

I'm not working on this at the moment; I'm a bit busy implementing other builtins.
@AryaveerSR Are you working on any method from this issue?

@AngeloChecked
Copy link
Contributor

Hi @jedel1043, Alright, I believe I can help with this now.

But i have a question for you: do you have for me any tips or guidelines for figuring out the type signatures of these functions?
From a less familiar perspective like mine, some of these can be a bit tricky. For example, could you confirm if my intuition is correct:

JsResult<Self> - (from sort, slice.. fn) Functions returning their own value, also with mutations.
JsResult<JsValue> - (from reduce.. fn) Used for very dynamic results
JsResult<Option<usize>> - (from index_of fn) Is Option used for handling potential undefined or -1 returns?

@jedel1043
Copy link
Member Author

For example, could you confirm if my intuition is correct:

Yep, that's pretty much on point. Our rule is just to use the type system as strictly as we can when returning values, so if a method only returns an index, we return usize, or if the method always returns a string, we return JsString.

@AngeloChecked
Copy link
Contributor

AngeloChecked commented Nov 9, 2023

Hi @jedel1043, sorry for bombarding you with questions: I'm using boa_examples/jstypedarray.rs for testing new functions. I found the entries function returning an iterator, and I need to make step and value functions of iterables module public instead of pub(crate).

/// boa_engine/src/object/builtins/jstypedarray.rs
/// Calls `TypedArray.prototype.entries()`.
pub fn entries(&self, context: &mut Context<'_>) -> JsResult<JsValue> {
    BuiltinTypedArray::entries(&self.inner.clone().into(), &[], context)
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/entries

/// boa_examples/src/bin/jstypedarray.rs
let mut result = vec![];
let data: Vec<u8> = vec![1, 2, 3];
let array = JsUint8Array::from_iter(data, context)?;
let mut iterator = array.entries(context)?.get_iterator(context, None, None)?;
while !iterator.step(context)? {
    let item = iterator.value(context)?.as_object().unwrap().clone();
    let js_array = JsArray::from_object(item)?;
    let index = js_array.get(0, context)?.as_number().unwrap();
    let value = js_array.get(1, context)?.as_number().unwrap();
    result.push((index, value))
}
assert_eq!(vec![(0f64, 1f64), (1f64, 2f64), (2f64, 4f64)], result);

https://github.com/boa-dev/boa/blob/main/boa_examples/src/bin/jstypedarray.rs

///boa_engine/src/builtins/iterables/mod.rs
/// [spec]: https://tc39.es/ecma262/#sec-iteratorstep
pub fn step(&mut self, context: &mut Context<'_>) -> JsResult<bool> {
    self.step_with(None, context)
}

/// Gets the current value of the `IteratorRecord`.
pub fn value(&mut self, context: &mut Context<'_>) -> JsResult<JsValue> {
    self.set_done_on_err(|iter| iter.last_result.value(context))
}

https://github.com/boa-dev/boa/blob/main/boa_engine/src/builtins/iterable/mod.rs#L421
Commonly can I make api public like this? Or do you have any tips on testing iterators or a better approach?

@jedel1043
Copy link
Member Author

@AngeloChecked I think you can skip iterator related methods for now. We haven't designed a good public API for Js iterators, so I'd prefer to wait until we have iterators done instead of rushing and exposing internal APIs that may as well change in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: To do
Development

No branches or pull requests

3 participants