Boa: [builtins - Object] Object.defineProperties() not implemented

[croraf]

Also, as per: https://github.com/boa-dev/boa/pull/543#discussion_r450857804 , after this is done, Object.create should be improved to handle second argument (the object properties) along with some tests.

[Razican]

Hi, this seems to be a new enhancement request, not something that is buggy. Could you please fill the template we have for this? There are no links to the spec or proper explanations on what this means.

[croraf]

Which explaination you need on "Object.defineProperties() not implemented"?

It is not clear which of these should be selected in this case. Perhaps "Feature request" description should be improved.

[Razican]

It is not clear which of these should be selected in this case. Perhaps "Feature request" description should be improved.

Done in #554.

Which explaination you need on "Object.defineProperties() not implemented"?

Do you have a link to the spec where this is explained? A code example that you would like to make work?

Also, you mention Object.create. What is this second parameter? Do you have a link to the spec? An example code? You mention we should add new tests. Which tests? why?

[croraf]

Do you have a link to the spec where this is explained? A code example that you would like to make work?

It should not be my job as a end-user and issue reported to suggest the way how to implement it, and provide suggestions where to find the information. The reporter should have a discretion to add this or not. The issue report is perfectly clear as is.

My extra effort here was that I checked that Object.defineProperties does not work because it is not implemented. I could have just said it is not working as in Chrome, so it would be a bug. It is not the mandatory job of the reported to search if some JS code does not work because "it is not implemented", or because "it is wrongly implemented".

Also, you mention Object.create. What is this second parameter? Do you have a link to the spec? An example code? You mention we should add new tests. Which tests? why?

Although I think this is also clear, I added a link to the related closed issue which provides more details.

[Razican]

It should not be my job as a end-user and issue reported to suggest the way how to implement it, and provide suggestions where to find the information. The reporter should have a discretion to add this or not. The issue report is perfectly clear as is.

Sure, but understand that the more detail you give, the more possibilities you have for us to take this into account.

In any case, let me add some information for future developers.

This is defined in the spec here. This should be added to the Object global object in this file, similarly to the Object.create() method.

Then, this needs to be added as a property of the Object in the end of the file, and we could take the opportunity to fix the Object.create() method above by taking into account the second parameter, instead of panicking.

Note that this Object.defineProperties() will require first calling on get_type() to assert the type of the object, but in fact, this can be a bit more efficient by doing an if let in the Value.

Then, in the spec, you call ToObject(), which is defined in the interpreter here.

Then it calls ToPropertyDescriptor() and DefinePropertyOrThrow(), which I think they are not defined. And in fact, we might not be properly using property descriptors. Maybe @HalidOdat or @jasonwilliams can give a hint.

[HalidOdat]

Then it calls ToPropertyDescriptor() and DefinePropertyOrThrow(), which I think they are not defined. And in fact, we might not be properly using property descriptor. Maybe @HalidOdat or @jasonwilliams can give a hint.

Yes. ToPropertyDescriptor() and DefinePropertyOrThrow() are not implemented, I planed to fix/implement (the internal methods are not spec compliant) them after #553

[jasonwilliams]

Now that the blockers are out the way this should be straight forward for someone to pick up

[dvtkrlbs]

i can take this issue

[dvtkrlbs]

Ok i am creating the Object.defineProperties function but i did not understand something in the spec.
Let keys be ? props.[[OwnPropertyKeys]](). what this means and is it defined already in the source code. I see these things are defined here but i dont understand if i should define them myself.

[croraf]

@dvtkrlbs I guess that should be a method of Rust's Object structure that seems not to be implemented, so should be implemented within this task or before this task. (Also you should join discord channel for boa for easier discussion. Link should be on the homepage)

[jasonwilliams]

Ok i am creating the Object.defineProperties function but i did not understand something in the spec.
Let keys be ? props.[[OwnPropertyKeys]](). what this means and is it defined already in the source code. I see these things are defined here but i dont understand if i should define them myself.

I imagine your "defineProperties" will be similar to this:
https://github.com/boa-dev/boa/blob/master/boa/src/builtins/object/mod.rs#L98-L105

Let keys be ? props.[OwnPropertyKeys].
This means loop through each key in the object's property list.
The ? just means each access shouldn't result in an abrupt ending