Flow: Flow doesn't understand calling a function to validate a property

[natew]

As shown here:

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjGBDAzrsADTAG9UwwAHAJzktwC4wB+AeQCMArAUwwBdyYatwB2AE27UAFAEomANzgBLMaUEU+ACyW4AdKNwBXYQAVa9WerAce-XQGtuAT1xStO3TTq4ZggL6CBsbcZt6yahQUSlBgUgCE7npe9DIRkRqatEgi3EgAotS00gDkInBU5rjFvukBFAEBmHAiuHxgAB5gALxgOUiE4ejtusLikpaoQA

[Naddiseo]

I think I ran into the same issue, although I manifested it in a different way. It seams that calling a method will invalidate the types. I edited your example, and it still fails:

/* @flow */
class X {
  props: ?Object
  render(): void {
    if (!this.props) { throw new Error("no props");  }
    // At this point, props should be checked as an object
    this.noop(); // XXX: For some reason this line invalidates the type checking
    // Next line is reported as an error that props could be null|undefined
    Object.keys(this.props);
  }
  noop() {}
}

const x = new X();
x.render()

Commenting out the call to this.noop() makes it succeed again.

example here

[mruzekw]

Flow's behavior makes sense to me in both of your cases.

@natew Flow doesn't crawl into a function for a null check.

@Naddiseo Flow doesn't know what happens to this.props after this.noop() is called. Thus it invalidates the null check.

[natew]

@mruzekw sure, it may not now. But it doesn't make sense to me in terms of "should".

In both cases you've written typesafe code and flow doesn't "see" it. To me, it seems like a flow bug.

[mruzekw]

Flow can only do so much without turning into a runtime engine 😬

[jcready]

Flow should be able to mark functions as “pure” internally and thus avoid the vast majority of refinement invalidations w/o needing a full runtime. It can already perform refinements using predicate functions with the undocumented %checks annotation. Flow just needs to get smarter about doing this automatically (and providing better documentation).

[mruzekw]

@jcready I think you're right. Flow could check if the function is pure or not without too much trouble. But wouldn't that only cover @Naddiseo's case?

[jcready]

@mruzekw it is possible to solve @natew's case by doing something like this:

const isObject = (x: mixed) : boolean %checks => typeof x === 'object' && !!x

class X {
  props: ?Object
  render(): void {
    if (isObject(this.props)) {
      Object.keys(this.props)
    }
  }
}

It looks to be impossible to define a method of a class to be a predicate using %checks, but that's just another thing Flow could improve upon.

[Naddiseo]

@mruzekw, I see your point. If flow doesn't follow the method call, there's no way for it to know if props were modified or not. However, I'd argue that's a bug or limitation of flow that should be addressed because adding superfluous if check seems unnecessary in the following:

/* @flow */

declare class Thing {
    noop(): void;
}

class X {
  props: null | Thing;
  constructor() {
    this.props = null;  
  }
  init(): void {
    if (this.props !== null) { throw new Error("already initiated");  }
    this.props = new Thing();

    this.props.noop(); // why does this invalidate props?

    //if (!this.props) { return; } // why is this necessary?
    this.props.noop();
    //if (!this.props) { return; } // why is this necessary?
    this.props.noop();

  }
}

const x = new X();
x.init()

flow/try link

[jcready]

@Naddiseo this is simply refinement invalidation. Even just calling console.log() will invalidate your prior refinements. I agree this isn't ideal, but it is documented.

[Naddiseo]

@jcready, thanks for pointing me at the docs!

[anisjonischkeit]

Just came across this issue in my code and after some experimenting came across this case where flow doesn't correctly invalidate the check.

// @flow
const data = { prop: { a: "bla" } }

function impureFunction() { 
  delete data.prop.a
}

function method(value: { prop?: { a: string } }) {
  if (value.prop) {
    const prop = value.prop

    impureFunction();

    // No flow error but a javascript error since a is undefined
    console.log(prop.a.charAt(0))
  }
}

method(data)

Am I missing something here?

[push1st1k]

@anisjonischkeit you need to define type for your 'data' at first line, like Try