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

Document "%checks" (Was: "if" condition type guard does not work when moved to function) #4723

Closed
lll000111 opened this issue Aug 26, 2017 · 23 comments

Comments

@lll000111
Copy link
Contributor

Description

I moved the boolean "if" condition into a function because it was pretty long. Without any other changes, this if-condition type guard is no longer recognized and Flow complains about using properties that that function checks exist.

After some experiments, I don't see a way to have a type guard be a function?

I understand the "mutability" assumption Flow makes whenever a function is called, but being unable to use a function as type guard, as seems to be the case, seems a little... severe. After all, functions starting with is...() (isBoolean, isThis, isThat, etc.) are quite common. I am unable to use a function in a type guard check?

Code

// @flow
'use strict';

function test1 () {
    // ERROR: Property 'myFn' cannot be assigned on 'data' (mixed)
    function isMyFunction (data: mixed): boolean {
        return typeof data === 'object' && data !== null && data.myFn === 'function';
    }

    function demo (data: mixed): void {
        if (isMyFunction(data)) {
            data.myFn = () => {
                // ...
            };
        }
    }
}

function test2 () {
    // WORKS
    function demo (data: mixed): void {
        if (typeof data === 'object' && data !== null && data.myFn === 'function') {
            data.myFn = () => {
                // ...
            };
        }
    }
}

Error

Error: src/main.js:11
 11:             data.myFn = () => {
                      ^^^^ property `myFn`. Property cannot be assigned on
 11:             data.myFn = () => {
                 ^^^^ mixed

Found 1 error

Working example

Flow Try link

@jcready
Copy link
Contributor

jcready commented Aug 26, 2017

Add %checks to your function annotation:

function isMyFunction (data: mixed): boolean %checks {

@lll000111 lll000111 changed the title An "if" condition type guard does not work when moved to function (e.g. "isFunction(..): boolen") An "if" condition type guard does not work when moved to function (e.g. "isFunction(..): boolean") Aug 26, 2017
@lll000111
Copy link
Contributor Author

lll000111 commented Aug 26, 2017

This leads to a question and a problem:

Question: Is this documented somewhere?

Problem: Since I'm not the only one who has never heard of that annotation Webstorm goes berserk (and all yellowish "I don't recognize what you are writing" background). Which leads back to the question - I don't want to file a Webstorm ticket for a feature that isn't documented.

@jcready
Copy link
Contributor

jcready commented Aug 26, 2017

I agree it should be documented, but to work around your problem you can do this:

function isMyFunction (data: mixed)/*: boolean %checks */{

@lll000111 lll000111 changed the title An "if" condition type guard does not work when moved to function (e.g. "isFunction(..): boolean") Document "%checks" (Was: An "if" condition type guard does not work when moved to function) Aug 27, 2017
@lll000111 lll000111 changed the title Document "%checks" (Was: An "if" condition type guard does not work when moved to function) Document "%checks" (Was: "if" condition type guard does not work when moved to function) Aug 27, 2017
@lll000111
Copy link
Contributor Author

lll000111 commented Aug 27, 2017

@vkurchatkin I changed the title, would you please reopen the issue and label it "documentation"? I think that's better than me opening another issue.

Could somebody tell me if %checks is official enough to open a Webstorm ticket to add this to the accepted syntax? Or is this just a workaround that some day may disappear.

@lll000111
Copy link
Contributor Author

lll000111 commented Aug 27, 2017

UPDATE:

The check does not work when I move it into another file (module) and require() it from there. The exact same function!!

@TrySound
Copy link
Contributor

It's expected. Only type annotations passes through files. This makes flow fast. Passing additional information with %checks can solve the problem, but this is not implemented yet.

@lll000111
Copy link
Contributor Author

@TrySound There is a problem - so I opened an issue.

@TrySound
Copy link
Contributor

#34

@lll000111
Copy link
Contributor Author

lll000111 commented Aug 27, 2017

@TrySound I read that yesterday. I searched for "%checked" in the issues list. I refrained from linking it because it isn't about %checks in particular. I tried to limit the scope of my issue to that concrete item, since as you correctly point out there already is one for more general discussion.

@roryokane
Copy link
Contributor

roryokane commented Sep 20, 2017

@Whoaa512
Copy link

Whoaa512 commented Oct 26, 2017

@panagosg7 since you implemented this could you tell me how I would use %checks in a library definition? From the playground, looks like it's not possible at the moment. Could it be?

e.g.

declare module "lodash" {
  declare class Lodash {
      isString: (value: any): boolean %checks (typeof value === 'string');
  }

  declare var exports: Lodash;
}

function method(x: string | number): number {
  if (lodash.isString(x)) {
    return x.charCodeAt(0);
  } else {
    return x;
  }
}

@panagosg7
Copy link
Contributor

panagosg7 commented Oct 30, 2017

I'm afraid function predicates (and the use of %checks modifier) are not supported for class methods at the moment.

facebook-github-bot pushed a commit that referenced this issue Nov 9, 2017
Summary:
For #4723
Closes #5087

Reviewed By: gabelevi

Differential Revision: D6069274

Pulled By: panagosg7

fbshipit-source-id: 397eb5623bc93a989a34dd4fa5f45808ab6e3e2b
@loyd
Copy link

loyd commented Nov 20, 2017

@Whoaa512 if import the check function by name:

import {isString} from 'lodash';

function method(x: string | number): number {
  if (isString(x)) { .... }
}

It works for me.

@Whoaa512
Copy link

@loyd Is that using the flow-typed declarations? or a custom one you wrote? Could you share that it looks like?

@loyd
Copy link

loyd commented Nov 22, 2017

@Whoaa512

I have tried not with lodash, but the same case:
Declaration
Import
Usage

@roryokane
Copy link
Contributor

Permanent links to the code linked in the previous comment: Declaration, Import, Usage

@loyd
Copy link

loyd commented Dec 1, 2017

Thanks, my mistake =(

@maxmalov
Copy link

maxmalov commented Mar 2, 2018

Do predicate functions work with instanceof I'm trying to implement a simple error handler with no luck frowtry. Or am I missing something else?

@panagosg7
Copy link
Contributor

Hi @maxmalov! instanceof works. The problem in the above is the property access. Flow cannot refine properties of objects in predicate functions. Here's a workaround: try-flow

@mrkev
Copy link
Contributor

mrkev commented Apr 23, 2018

Checks is documented here.

@akoppela
Copy link

I'm afraid function predicates (and the use of %checks modifier) are not supported for class methods at the moment.

@panagosg7 Do you know if function predicates support was added for class methods?

@panagosg7
Copy link
Contributor

@akoppela No, class methods are still not supported.

@danielo515
Copy link

How would you use checks to say that a function is able to determine of an object is of certain type? A type you defined yourself like type Point = { x: number, y: number} ?

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

No branches or pull requests