Flow: do property not found in object literal errors really help anyone?

Created on 31 Mar 2016  路  22Comments  路  Source: facebook/flow

Maybe this has been fixed since Flow 0.20.1 but...

/* @flow */

function createSelector(options: Object): Object {
  let selector = {
    beginTimestamp: {$lt: options.timeRange.end},
    endTimestamp: {$gt: options.timeRange.begin}, 
  };

  if (options.location && options.location !== 'all') {
    selector.location = options.location;
  }

  return selector; 
}
$ flow status
react-sandbox/scratch/flowissues.js:10
 10:     selector.location = options.location;
                  ^^^^^^^^ property `location`. Property not found in
 10:     selector.location = options.location;
         ^^^^^^^^ object literal


Found 1 error

Seriously, how does this help? I think this just wastes developer time. Tacking new properties onto objects, or deleting properties, are valid JS operations and don't contradict the declared return type here, so what gives?

JS is dynamically typed, the MongoDB API is dynamically typed, and therefore tacking new properties onto objects (even deleting properties sometimes) will always remain convenient, more convenient than Flow trying to help prevent whatever could hypothetically go wrong, in my opinion.

Most helpful comment

@oliviertassinari yes, you can add : Object:

let selector: Object = {
  ...
}

selector.hello = 'world'

I don't think the : Object should be necessary though.

All 22 comments

Any idea how we can disable this type of warnings?

@oliviertassinari yes, you can add : Object:

let selector: Object = {
  ...
}

selector.hello = 'world'

I don't think the : Object should be necessary though.

This is problematic when defining an object separate from its variable declaration:

function handler(req, res) {
  let obj: Object;
  if (req.body) {
    obj = { a: 1 };
    if (req.body.whatever) {
      obj.b = 2;
/*
7:       obj.b = 2;
             ^ property `b`. Property not found in
7:       obj.b = 2;
         ^ object literal
*/
    }
  } else {
    throw new Error('missing request body');
  }

  res.send(obj);
}

https://flowtype.org/try/#0PTAEAEDMBsHsHcBQkCuA7AxgFwJazaABYCGaAJtAKYBOAFNZQI4A0oDAzgJSgDeiooKllCwARgCsAXKADyEytgDc-UDkih6TAHSjYZAJ7c+AgWPGgAvL1DFpARlABfZSdXrNjHXv1b4JLJQAbjRGKq5mOpagAEwuJo4qjqCU0OyUvGGgWITUCKBolPCgAKLUuXQA5AC2OOzsOGgA5mxMKJTswroGFZxxCQIqHFpp5LRmvYgJQA

Creating a clone of the object or defining a new temporary variable both work so this isn't a show-stopper but it is a scenario where the programmer has to adopt to Flow instead of the other way around, which Flow is generally so much better at than other systems.

@oliviertassinari These days I often just cast to Object when tacking new properties on:

  const selector = {
    beginTimestamp: {$lt: options.timeRange.end},
    endTimestamp: {$gt: options.timeRange.begin}, 
  }

  if (options.location && options.location !== 'all') {
    (selector: Object).location = options.location
  }

@ide you could do (obj: Object).b = 2

@jedwards1211 Thanks for the tips.

There are two options to deal with this problem:

  • explicit annotation:
const obj: { a: number, b?: string } = { a: 1 };
obj.b = 'foo';

We mark b as optional so it can be added later

  • unsealed object
const obj = {};
obj.a = 1;
obj.b = 'foo';

When you create an empty object it is "unsealed". This means that you can add any properties you want. This behaviour is not 100% safe, so one should be careful with it.

@vkurchatkin Okay, so in the case of my OP, the error is completely intentional and not going to change?

I was hoping that at least in this case since the added properties don't contradict the return type (which is all the object is being used for) that eventually flow could stop giving this error.

@jedwards1211 I agree, this behaviour is safe, pretty common, and ideally should be supported

you can also avoid having to explicitly annotate the entire object by pre-declaring the optional properties as undefined, but the catch is that you have to annotate those undefineds to widen them because they won't be allowed to be widened later:

Example

const x = {
  a: 'foo',
  b: (undefined: string | void),
  c: (undefined: string | void)
};
if (Math.random() > 0.5) {
  x.b = 'foo'; // ok
  x.c = 123; // error
}

I ran into an unusual manifestation of this problem recently where the same syntax would pass in one file but fail in another.

// Works in file A.jsx:

    this._infoBoxStyle.display = {
      [ActionStatus.FATAL]: 'block',
      [ActionStatus.WARNING]: 'block',
      [ActionStatus.RECOVERED]: 'none',
    }[this.props.appStatus];

//  Fails in file B.jsx:

    this._displayStatus = {
      [ActionStatus.FATAL]: error,
      [ActionStatus.WARNING]: error,
      [ActionStatus.RECOVERED]: good,
    }[this._status] || good;

// error:

125:     }[this._status] || good;
           ^^^^^^^^^^^^ property `Fatal`. Property not found in
121:     this._displayStatus = {
                               ^ object literal

// The fix:

    this._displayStatus = ({
      [ActionStatus.FATAL]: error,
      [ActionStatus.WARNING]: error,
      [ActionStatus.RECOVERED]: good,
    }: Object)[this._status] || good;  // https://github.com/facebook/flow/issues/1606

I copied and pasted the statement from A into B and it fails in B as well. I then copied the statement from B into A and it passes in A!
Anyone know why it would pass in one file but not another? Both files were just defining react components, not doing anything differently that I can discern.

@colinhunt I'm not sure that's related to my OP, because it doesn't involve adding new properties to objects created by object literals

The fix suggested by @jedwards1211 fixes this though. That's why I added it here.

@colinhunt yes, but that's just because Object is a very generic supertype, and casting to Object eliminates all shape type errors

Do you think I should create a new issue?

@colinhunt yes, but I think the issue might lie in the types of this._infoBoxStyle, this._displayStatus, this.props.appStatus, and this._status -- the syntax might be the same but there might be differences in the type defs for those variables. You might want to ask the same question with that additional info on Stack Overflow first. Or you're welcome to PM me with that.

So when I add a function to a prototype I must always cast it down to Object?

(Rx.Observable: Object).prototype.bufferIntervalCount = function (options, scheduler) { ... }

@memelet Yes that seems to be right.

IMO there really needs to be a configuration parameter for this, I'm using // $FlowFixMe whenever I add a field to an object literal.. When I want this behavior for an object, I use Object.seal().

so if I suppose to add custom fields to some errors, creating a class for same kind for error objects is better than just use err: Object?

We understand that this is not ideal coming from JavaScript, but this is the tradeoff we make in designing a type system for JavaScript. We will never be able to effectively type 100% of JavaScript as it would be way too expensive to think about every case and the type system may need to be looser and make less guarantees.

To fix the original issue, please refer to @vkurchatkin鈥檚 solutions which are what we recommend. If you have a specific case you would like to support please open a new issue.

I know this issue has been closed but in my opinion casting to generic Object is a poor fix for this that completely negates any advantages of flow. More often than not, even when assigning properties to an object, you want at least some properties guaranteed. In the original example, the OP listed beginTimestamp and endTimestamp as two such properties.

Rather than being all or nothing, I'd like to see a non-intrusive way to tell flow that other arbitrary properties can be set on this object, using spread operator without operand, for example:

interface FooBar {
    foo: number;
    bar: number;
}
interface FooBarBaz {
    foo: number;
    bar: number;
    ...
}
let object1: FooBar = { foo: 1, bar: 2};
let object2: FooBarBaz = { foo: 1, bar: 2};
object1.baz = 3; // error
object2.baz = 3; // no error
Was this page helpful?
0 / 5 - 0 ratings