Some issues with the checkings

[Fernando-Marquardt]

Hello, I may have found a problem with the is-data-descriptor and is-accessor-descriptor checkings.

A data property descriptor is one that includes any fields named either [[Value]] or [[Writable]]. An accessor property descriptor is one that includes any fields named either [[Get]] or [[Set]]. Any property descriptor may have fields named [[Enumerable]] and [[Configurable]]. A Property Descriptor value may not be both a data property descriptor and an accessor property descriptor; however, it may be neither.
-- The Property Descriptor and Property Identifier Specification Types

This one works fine:

// object: { test: 'value' }
var descriptor =  {
    value: 'value',
    writable: true,
    enumerable: true,
    configurable: true
};

isDataDescriptor(descriptor ); // true - right
isAccessor(descriptor ); // false - right

But, if we do this:

/*
object: {
    get test() {
        // Return something
    },

    set test(value) {
        // Do something
    }
}
*/
var descriptor = {
    get: [Function: test],
    set: [Function: test],
    enumerable: true,
    configurable: true
};

isDataDescriptor(descriptor); // true - wrong
isAccessor(descriptor); // true - right

Which is wrong, because it doesn't have neither the [[value]] or [[writable]] attributes.

This one { get: [Function: test], set: undefined, enumerable: true, configurable: true } is not even considered a descriptor either. But, as an accessor descriptor it can have either [[get]] or [[set]] and should return false only if doesn't have any, as you can read here:

When the abstract operation IsAccessorDescriptor is called with property descriptor Desc, the following steps are taken:

1. If Desc is undefined, then return false.
2. If both Desc.[[Get]] and Desc.[[Set]] are absent, then return false.
3. Return true.

-- IsAccessorDescriptor ( Desc )

[jonschlinkert]

I may have found a problem with the is-data-descriptor and is-accessor-descriptor checkings.

It looks like a single issue with is-data-descriptor, possibly how this library implements is-data-descriptor.

the solution is that we need to ensure that is-data-descriptor checks for value or writable. correct?

thanks for bringing it to my attention

[Fernando-Marquardt]

Yes, is-data-descriptor must check for the existence of the fields value or writable. as you said. If both doesn't exist, it can't be a DataDescriptor, but it may still be a Descriptor, because there is the GenericDescriptor, but I don't know if there is need to go that deep.

But the problem with is-accessor-descriptor occurs when you have the getter but not the setter. In this case, the function returns false, but it should return true, because as you can read there, the accessor descriptor will be valid if it includes the fields get or set.

Well, I agree the other way around doesn't make sense and you shouldn't have a setter but not a getter (jshint even warns you about that) but it is still valid for Javascript =/