Closed Bug 1245024 Opened 8 years ago Closed 8 years ago

[es7] implement Object.getOwnPropertyDescriptors() proposal

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: andrea.giammarchi, Assigned: evilpie)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])

Attachments

(3 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36

Steps to reproduce:

Apologies if this is not the best channel but it's already happening in Chrome so I've thought to file a bug/request in here too. The Chromium related issue is the following one: https://codereview.chromium.org/1658773003/


Actual results:

Object.getOwnPropertyDescriptors is not defined


Expected results:

There is a stage 3 proposal waiting to be promoted to stage 4: https://github.com/tc39/proposal-object-getownpropertydescriptors#objectgetownpropertydescriptors-proposal-polyfill

Expecting Object.getOwnPropertyDescriptors to be available and return a generic object with all own properties (names + Symbols) and relative descriptors as described in the proposal.

A polyfill-like version of what should happen is described in here:
https://github.com/tc39/proposal-object-getownpropertydescriptors#proposed-solution

Thanks in advance for considering this change or for redirecting this to the right team.

Best Regards
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
No review request yet, because I have to look into importing https://github.com/tc39/test262/tree/master/test/built-ins/Object/getOwnPropertyDescriptors somehow.
Assignee: nobody → evilpies
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
This is not yet in the current spec at http://tc39.github.io/ecma262. There is still some discussion about the case when the descriptor returned by [[GetOwnProperty]] is undefined: https://github.com/tc39/ecma262/pull/582
https://github.com/tc39/ecma262/pull/582 has now been merged unchanged. In July, we'll get committee consensus on the additional "if" check in https://github.com/tc39/ecma262/pull/593 which only applies to Proxies.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e402e788d6f&selectedJob=21987444
dom/plugins/test/mochitest/test_defaultValue.html is failing, I suspect we are passing a symbol to function that can't handle it. As usual, just running the test locally doesn't actually reproduce it.
Attachment #8756410 - Flags: review?(jorendorff)
I made a few changes to these tests to avoid having to import a lot of code from the test suite.
Attachment #8760090 - Flags: review?(jorendorff)
Comment on attachment 8756410 [details] [diff] [review]
Implement Object.getOwnPropertyDescriptors

Review of attachment 8756410 [details] [diff] [review]:
-----------------------------------------------------------------

Seems totally fine. Can you file a followup to track the spec change that's in the works for thi stage 4 proposal? I mean the extra if, which Jordan says only affects the proxy case of returns from [[GetOwnProperty]].
Attachment #8756410 - Flags: review?(jorendorff) → review+
Comment on attachment 8760090 [details] [diff] [review]
Port tests from tests262

Review of attachment 8760090 [details] [diff] [review]:
-----------------------------------------------------------------

Hmmm, I wonder about putting these in the test262 directory, which I don't think is ever run. Is there somewhere we can put them where they will be run for sure? add a bunch fo reportCompare() calls and put them in js/tests/ for example?

r=me with assurances that they are actually being run in automation.
Attachment #8760090 - Flags: review?(jorendorff) → review+
Going to merge this before landing.
Comment on attachment 8761660 [details] [diff] [review]
Add getOwnPropertyDescriptors to test_xray

r=me
Attachment #8761660 - Flags: review+
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5238e71b3635
Implement Object.getOwnPropertyDescriptors. r=efaust,bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/06a54c686fc9
Import test262 tests. r=efaust
Blocks: 1279303
https://hg.mozilla.org/mozilla-central/rev/5238e71b3635
https://hg.mozilla.org/mozilla-central/rev/06a54c686fc9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.