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

Object.defineProperties must not throw when provided undefined in place of a descriptor #652

Closed
claudepache opened this issue May 27, 2016 · 4 comments

Comments

@claudepache
Copy link
Contributor

claudepache commented May 27, 2016

Apparently, a change between ES5 and ES6 not reflected in the test suite, (neither in the few implementations I have checked, BTW).

test/built-ins/Object/defineProperties/15.2.3.7-5-b-1.js contains:

var obj = {};
assert.throws(TypeError, function() {
            Object.defineProperties(obj, {
                prop: undefined
            });
});

(I have not checked the numerous other tests about the same step.)

However, according to ecma262#sec-objectdefineproperties Step 5.b, undefined values must not throw, but be ignored.

@leobalter
Copy link
Member

leobalter commented May 27, 2016

It properly throws a TypeError when you call ToPropertyDescriptor passing an undefined value:

6.2.4.5 ToPropertyDescriptor ( Obj )

1. If Type(Obj) is not Object, throw a TypeError exception.
...

---

19.1.2.3.1 Runtime Semantics: ObjectDefineProperties ( O, Properties )

1. If Type(O) is not Object, throw a TypeError exception.
2. Let props be ? ToObject(Properties).
3. Let keys be ? props.[[OwnPropertyKeys]]().
4. Let descriptors be a new empty List.
5. Repeat for each element nextKey of keys in List order,
  a. Let propDesc be ? props.[[GetOwnProperty]](nextKey).
  b. If propDesc is not undefined and propDesc.[[Enumerable]] is true, then
    i. Let descObj be ? Get(props, nextKey).
    ii. Let desc be ? ToPropertyDescriptor(descObj).
...
---

var o = {};
var properties = { prop: undefined };

Object.defineProperties(o, properties);
// props = properties
// keys = [ 'prop' ]
// keys => each( nextKey ):
//   nextKey = 'prop'
//   propDesc = props.[[GetOwnProperty]]('prop')
//   propDesc = {value: undefined, writable: true, enumerable: true, configurable: true}
//   descObj = Get(props, 'prop')
//   descObj = undefined
//   desc = ToPropertyDescriptor(undefined)
//     => descObj((undefined)) is not Object, throw a TypeError exception.

@claudepache
Copy link
Contributor Author

There was some confusion of my part: In step 5b of DefinePropertyDescriptors, the condition is propDesc is undefined, and not propDesc.[[Value]] is undefined. Sorry.

@leobalter
Copy link
Member

It is confusing as it catches the prop descriptors to find the enumerables
properties.

Btw, propDesc.value is undefined, but [[Value]] is the value of propDesc.

On Friday, May 27, 2016, Claude Pache notifications@github.com wrote:

There was some confusion of my part: In step 5b of
DefinePropertyDescriptors, the condition is propDesc is undefined, and
not propDesc.[[Value]] is undefined. Sorry.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#652 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AASYkXtJ6ZNaIMFi_8k5bA2WmCnZtY8qks5qFtBSgaJpZM4IoPj6
.

@leobalter
Copy link
Member

Or no, I'm in a train and will revise my last comment in a better moment.

On Friday, May 27, 2016, Leo Balter leonardo.balter@gmail.com wrote:

It is confusing as it catches the prop descriptors to find the enumerables
properties.

Btw, propDesc.value is undefined, but [[Value]] is the value of propDesc.

On Friday, May 27, 2016, Claude Pache <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

There was some confusion of my part: In step 5b of
DefinePropertyDescriptors, the condition is propDesc is undefined, and
not propDesc.[[Value]] is undefined. Sorry.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#652 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AASYkXtJ6ZNaIMFi_8k5bA2WmCnZtY8qks5qFtBSgaJpZM4IoPj6
.

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

No branches or pull requests

2 participants