Flow: Object.defineProperty with a property descriptor without "value" causes an error

[kwkbtr]

When I try to use Object.defineProperty with an accessor property descriptor, flow check shows an error.
It seems that using Object.defineProperty with a property descriptor without value property fails to typecheck.

/* @flow */
var x = { foo: 1 };

// this typechecks
Object.defineProperty(x, "foo", {
    value: 1,
    writable: false
});

// this does not
Object.defineProperty(x, "foo", {
    writable: false
});
// error message:
// Object.defineProperty
// Property not found in
// (position of the property descriptor argument): object literal

// this neither
Object.defineProperty(x, "foo", {
    get: function() { return 1; }
});
// error message is the same as the above

Is this behavior intentional?

[jeffmo]

Indeed it looks like Flow currently special-cases value properties in the descriptor:
https://github.com/facebook/flow/blob/master/src/typing/type_inference_js.ml#L3651

It's definitely worth looking into support for more of the descriptor features than just value.

[bjouhier]

+1 for a fix. I'm hitting the problem with get descriptors.

[vjeux]

I'm running into it when trying to type this call inside of React
https://github.com/facebook/react/blob/master/src/shared/utils/canDefineProperty.js#L17

src/shared/utils/canDefineProperty.js:18
 18:     Object.defineProperty({}, 'x', {get: function() {}});
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Object.defineProperty. Property not found in
 18:     Object.defineProperty({}, 'x', {get: function() {}});
                                        ^^^^^^^^^^^^^^^^^^^^ object literal

[zertosh]

Please fix :(

[gaperton]

Wanna workaround?

Object.defineProperty(x, "foo", ({
    get: function() { return 1; }
} : Object ));

[dfleury]

I'm suffering with it 😢

[HerringtonDarkholme]

A real use case here:
https://github.com/vuejs/vue/pull/5132/files

[mbrowne]

A related discussion here: https://github.com/facebook/flow/issues/1336

IMO if unsafe.enable_getters_and_setters works with object literal syntax for getter/setters, it should work for defineProperty() too.

@gaperton thanks for mentioning the workaround, but does this accomplish anything other than suppressing the error (as can be done using suppress_comment)? It doesn't seem to actually type-check, e.g. if you add (x.foo: number) afterwards, it says "No errors" instead of "This type is incompatible with...".

[mbrowne]

@lll000111 the problem with your example is that you're using arrow functions, so this is lexically scoped. (If you run it inside a function in strict mode, it won't even work, since this would be undefined in that case.) This works:

const o = {};

Object.defineProperty(o, 'foo', ({
    enumerable: true,
    get: function() {
        return this.foo;
    },
    set: function(n) {
        this.foo = n;
    }
}: Object));

To be clear, I still don't like the workaround of casting to Object, which only suppresses the error (which you could alternatively do with $FlowFixMe comments).

[lll000111]

@mbrowne That is correct, I updated my issue (https://github.com/facebook/flow/issues/4742).

[smalllong]

I wonder why this hasn't been fixed yet

[borela]

No solution yet other than suppressing the error?

[julienw]

Current error in v0.70 is: Property value is missing in _object literal_.

testcase

[julienw]

My workaround is:

const defineProperty = Object.defineProperty;
defineProperty(obj, 'key', {
  enumerable: true,
  get() {
    return "something";
  },
});

It looks like this is enough to avoid the special case, but still profiting from the typing in lib/core.js.

[gaperton]

thanks for mentioning the workaround, but does this accomplish anything other than suppressing the error

Of course, it doesn't. That's why it's called a workaround. But it let your code to be compiled, which is a big deal.

[julienw]

For anyone following here, my workaround above switches off any special handling of Object.property, such as the code from this try. It's fine for my usage but maybe not for yours.