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

t.throw flow type check is giving a false-positive (Callable signature not found in) #1148

Closed
gajus opened this issue Dec 8, 2016 · 5 comments · Fixed by #1219 or singapore/gl-got#13
Labels
bug current functionality does not work as desired help wanted

Comments

@gajus
Copy link

gajus commented Dec 8, 2016

Description

I am getting an "Callable signature not found in" when using t.throws.

The type error originates in the node_modules/ava/index.js.flow definition.

This could be simply me misunderstanding what the error is saying.

I'd expect Flow to pickup:

throws(value: Promise<mixed>, error?: ErrorValidator, message?: string): Promise<mixed>;

annotation and ignore:

throws(value: () => void, error?: ErrorValidator, message?: string): mixed;

I even explicitly cast the result to Promise<void>.

If I remove the Promise<void> cast, I am getting a different error, https://gist.github.com/gajus/5c42f0262874a2bae7842691eea28a60#file-ava-flow-sh-L2.

Test Source

test('timesout after 60 seconds', async (t): Promise<void> => {
  const {
    document
  } = createDocument();

  global.document = document;

  const result: Promise<void> = loadScript('http://dailymail.co.uk/');

  clock.tick(6e4 + 1);

  t.throws(result, 'Script loading time has exceeded the maximum configured time limit.');
});

Error Message & Stack Trace

$ flow
node_modules/ava/index.js.flow:71
 71: 	throws(value: () => void, error?: ErrorValidator, message?: string): mixed;
     	              ^^^^^^^^^^ function type. Callable signature not found in
 94:   t.throws(result, 'Script loading time has exceeded the maximum configured time limit.');
                ^^^^^^ Promise. See: test/utilities/loadScript.js:94


Found 1 error

Config

Copy the relevant section from package.json:

{
  "ava": {
    "babel": {
      "plugins": [
        "transform-flow-strip-types",
        "transform-async-to-generator",
        "transform-es2015-modules-commonjs"
      ]
    },
    "require": [
      "babel-register"
    ]
  }
}

Command-Line Arguments

ava

Environment

Tell us which operating system you are using, as well as which versions of Node.js, npm, and AVA. Run the following to get it quickly:

Node.js v7.2.1
darwin 16.1.0
0.17.0
3.10.10
@jamestalmage
Copy link
Contributor

pinging @thejameskyle

@gajus
Copy link
Author

gajus commented Dec 12, 2016

Can this be marked as a bug?

I am not sure how to fix it... I would raise a PR otherwise.

It is breaking our integration tests. Annoyingly, I cannot even use FlowFixMe comment because it then throws error in ava definition. Therefore, we had to comment out async tests just to make Flow stop complaining about these assertions.

@sindresorhus sindresorhus added bug current functionality does not work as desired help wanted labels Dec 12, 2016
@gajus
Copy link
Author

gajus commented Feb 1, 2017

@leebyron Could you have a look at this issue please?

(Since you have tackled a similar issue here #1164.)

@leebyron
Copy link
Contributor

leebyron commented Feb 2, 2017

Taking a look - I'm not 100% sure what's wrong, but the js.flow file seems like it might be double-defining the throws type.

@leebyron
Copy link
Contributor

leebyron commented Feb 2, 2017

A simpler repro case is simply:

t.throws(Promise.reject(new Error()))

Which yields:

index.js.flow:61
 61: 	throws(value: () => void, error?: ErrorValidator, message?: string): mixed;
     	              ^^^^^^^^^^ function type. Callable signature not found in
 13:   t.throws(Promise.reject(new Error()))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Promise. See: test.js.flow:13

leebyron added a commit to leebyron/ava that referenced this issue Feb 2, 2017
Instead of double-defining each of these, define them once while using overloading.

Also took some liberties to improve the definition. Specifically, ava doesn't care what values these throwing functions return, but doesn't produce a return value.

Fixes avajs#1148
leebyron added a commit to leebyron/ava that referenced this issue Feb 2, 2017
Instead of double-defining each of these, define them once while using overloading.

Also took some liberties to improve the definition. Specifically, ava doesn't care what values these throwing functions return, but doesn't produce a return value.

Fixes avajs#1148
leebyron added a commit to leebyron/ava that referenced this issue Feb 3, 2017
Instead of double-defining each of these, define them once while using overloading.

Also took some liberties to improve the definition. Specifically, ava doesn't care what values these throwing functions return, but doesn't produce a return value.

Fixes avajs#1148
sindresorhus pushed a commit that referenced this issue Feb 5, 2017
Instead of double-defining each of these, define them once while using overloading.

Also took some liberties to improve the definition. Specifically, ava doesn't care what values these throwing functions return, but doesn't produce a return value.

Fixes #1148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug current functionality does not work as desired help wanted
Projects
None yet
4 participants