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

Access to PropertyMap keys #3830

Closed
zcourts opened this issue Apr 27, 2024 · 2 comments · Fixed by #3832
Closed

Access to PropertyMap keys #3830

zcourts opened this issue Apr 27, 2024 · 2 comments · Fixed by #3832
Labels
enhancement New feature or request

Comments

@zcourts
Copy link

zcourts commented Apr 27, 2024

Background

I've seen #1974 and #1961.
I'm having the same issue, I understand the explanation in the ticket as well, makes sense.

That being said, we're allowing any JsValue to be returned back to Rust land and the current API makes it a bit hard to traverse the object tree returned.

Is there a workaround currently?

Yes, you can combine OrdinaryObject::keys with JsArray::try_from_js to iterator over them and call JsObject::get with each one.

So, what's the problem?

Within the boa crate it can be done with one line of code, it is nice and simple...externally, it needs almost 30 lines.

Within Boa

obj.borrow().properties().shape.keys()

Externally

 let keys = OrdinaryObject::keys(
                    &JsValue::undefined(),
                    &[JsValue::Object(obj.clone())],
                    context,
                )?;
                let keys = JsArray::try_from_js(&keys, context)?;
                let mut out = Map::new();
                let mut i: i32 = 0;
                let len = keys.length(context)? as i32;
                while let Ok(key) = keys.at(i, context) {
                    let key = match key {
                        JsValue::Null => continue,
                        JsValue::Undefined => continue,
                        JsValue::Boolean(k) => {
                            PropertyKey::String(if k { "true".into() } else { "false".into() })
                        }
                        JsValue::String(k) => PropertyKey::String(k),
                        JsValue::Rational(k) => PropertyKey::String(k.to_string().into()),
                        JsValue::Integer(k) => PropertyKey::String(k.to_string().into()),
                        JsValue::BigInt(k) => PropertyKey::String(k.to_string().into()),
                        JsValue::Object(_) => continue,
                        JsValue::Symbol(_) => continue,
                    };
                    if obj.has_own_property(key.clone(), context)? {
                        let val = /*map*/obj.get::<PropertyKey>(key.clone(), context)?;
                        out.insert(key.to_string(), js_to_json(val, context)?);
                    }
                    i = i + 1;
                    if i >= len {
                        break;
                    }
                }

What's being requested?

A JsObject.keys() or PropertyMap.keys() method which return the keys on a JsObject so that we can iterate over the keys and call .get on the JsObject.

Within boa it can just be the snippet above that to_json uses.

Anything else?

  • As a new boa user it was incredibly difficult to work out how to do this. The example above of how I ended up doing it is just the final thing I ended up with
  • I had a variant where I tried to get a JsMap from obj but it fails saying obj is not a map...I don't know the ECMAScript spec but it seems to me that I should be able to convert a JsObject to a JsMap
  • To work around the above I tried something like this:
                 let obj_copy = JsObject::from_proto_and_data(
                     context.intrinsics().constructors().map().prototype(),
                     OrderedMap::<JsValue>::new(),
                 );
                obj_copy.copy_data_properties::<PropertyKey>(
                     &JsValue::from(obj.clone()),
                     vec![],
                     context,
                 )?;
let map = JsMap::from_object(obj_copy)?;
let keys = map.keys(context)?;
while let Ok(key) = keys.next(context)?
                {
//use key here to get value
}

this failed spectacularly as it appears keys.next(context) never fails and lead to an infinite loop. I had a look and couldn't find any tests or usages of the JsMapIterator returned. It returns a JsResult<JsValue> so it never ends unless there's a failure?

@zcourts zcourts added the enhancement New feature or request label Apr 27, 2024
@HalidOdat
Copy link
Member

Hi @zcourts 👋

I believe that #3832 should address this problem, see example

Let me know if there any issues with the API :)

@zcourts
Copy link
Author

zcourts commented Apr 28, 2024

Hi @HalidOdat 👋🏾 !
Yes, that's exactly what I needed. Amazing turn around time as well.

FYI it seems we've committed to boa over alternatives now after evaluating a few options so in future I'm happy to contribute changes like this after some guidance (I wouldn't have thought to look at __own_property_keys__ or that own_property_keys was the way to go

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

Successfully merging a pull request may close this issue.

2 participants