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

ava 0.17.0 with FlowType 0.35.0 results in type-ambiguity errors #1114

Closed
jokeyrhyme opened this issue Nov 18, 2016 · 9 comments · Fixed by #1164
Closed

ava 0.17.0 with FlowType 0.35.0 results in type-ambiguity errors #1114

jokeyrhyme opened this issue Nov 18, 2016 · 9 comments · Fixed by #1164
Labels
bug current functionality does not work as desired help wanted

Comments

@jokeyrhyme
Copy link
Contributor

jokeyrhyme commented Nov 18, 2016

Description

Trying to update a project to use ava 0.17.0

For what seem like pretty standard-looking ava tests to me (as far as I know), FlowType is getting all up in my face about some ambiguity relating to the new types that ava declares. I am obviously quite scandalised by all this. Oh my!

Test Source

This is the test code in question:

test('A, A->B, B->C, BC->end', (t) => {
  const tasks = [
    { fn: noop, id: 'a', label: 'a', requires: [ 'B', 'C' ] },
    { fn: noop, id: 'b', label: 'b', provides: [ 'C' ], requires: [ 'B' ] },
    { fn: noop, id: 'c', label: 'c', provides: [ 'B' ], requires: [ 'A' ] },
    { fn: noop, id: 'd', label: 'd', provides: [ 'A' ] }
  ]
  t.notThrows(() => tasks.sort(compareByMetadata))
  const ids = tasks.map((task) => task.id)
  t.deepEqual(ids, [ 'd', 'c', 'b', 'a' ])
})

Error Message & Stack Trace

When I run flow_check, I get 23 errors that are all very similar, like this:

test/sort-tasks.js:63
 63: test('A, A->B, B->C, BC->end', (t) => {
     ^ function call. Could not decide which case to select
201: declare module.exports: {
                             ^ callable object type. See: node_modules/ava/index.js.flow:201
  Case 2 may work:
  203: 	(name: string, run: ContextualTest): void;
       	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function type. See: node_modules/ava/index.js.flow:203
  But if it doesn't, case 4 looks promising too:
  205: 	(name: string, run: Macros<ContextualTestContext>, ...args: Array<any>): void;
       	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function type. See: node_modules/ava/index.js.flow:205
  Please provide additional annotation(s) to determine whether case 2 works (or consider merging it with case 4):
   63: test('A, A->B, B->C, BC->end', (t) => {
                                            ^ return

Config

Copy the relevant section from package.json:

{
  "ava": {}
}

Command-Line Arguments

Copy your npm build scripts or the ava command used:

nyc ava

Relevant Links

Environment

Node.js v7.1.0
darwin 16.3.0
ava 0.17.0
npm 4.0.2
@sindresorhus
Copy link
Member

@thejameskyle halp

@jokeyrhyme
Copy link
Contributor Author

@sindresorhus that kitten slays me every time. So cute! Your participation in any GitHub conversation is a welcome presence. :D

@jfmengels
Copy link
Contributor

I am getting the same error and can't seem to find a solution to it (except removing the macro possibility...) 🤔

@jokeyrhyme
Copy link
Contributor Author

I did get this working by explicitly annotating the types, something like this:

/* :: import type { ContextualTest } from 'ava' */
test('my test', (t) => { t.pass('yay') } /* : ContextualTest */)

But I don't want to have to do this for every single test.

Another interim solution would be to tell FlowType to ignore ava. /shrug

@jokeyrhyme
Copy link
Contributor Author

Just FYI: I'm having this issue with FlowType 0.36.0, too.

@vadimdemedes vadimdemedes added bug current functionality does not work as desired help wanted labels Nov 26, 2016
@mymattcarroll
Copy link

Another FYI: Having the same issue with flow-bin@0.37.0

leebyron referenced this issue in leebyron/testcheck-js Dec 23, 2016
leebyron added a commit to leebyron/ava that referenced this issue Dec 23, 2016
Fixes avajs#1114

As reported in avajs#1114, flow reports ambiguity when defining tests:

```js
test('my test', t => {})
```

Flow will report that it could apply either the `Test` type or the `Macro` type. The `Test` type is an obvious fit, but `Macro` also applies since the `...args` resolves to the empty array, and the `title?` attribute happens to be not provided.

My suggested fix is to merge the `Test` and `Macro` types together since `Macro` is really a superset of the `Test` type, and we don't lose any type safety by just expecting to always get Macros.

I tested this locally on a flow-typed repository and confirmed that the type ambiguity errors are solved.
leebyron added a commit to leebyron/ava that referenced this issue Dec 24, 2016
Fixes avajs#1114

As reported in avajs#1114, flow reports ambiguity when defining tests:

```js
test('my test', t => {})
```

Flow will report that it could apply either the `Test` type or the `Macro` type. The `Test` type is an obvious fit, but `Macro` also applies since the `...args` resolves to the empty array, and the `title?` attribute happens to be not provided.

My suggested fix is to merge the `Test` and `Macro` types together since `Macro` is really a superset of the `Test` type, and we don't lose any type safety by just expecting to always get Macros. This super set type is called `TestImplementation` which better matches how AVA names these things.

I tested this locally on a flow-typed repository and confirmed that the type ambiguity errors are solved.
mizchi added a commit to mizchi/frontend-boilerplate that referenced this issue Dec 31, 2016
@jokeyrhyme
Copy link
Contributor Author

Oooo, nice work! Yay! <3

@btipling
Copy link

Is this released yet? I still get this problem.

@jokeyrhyme
Copy link
Contributor Author

@btipling most recent release looks like 0.17.0 2016-11-17, whilst this was only merged 2016-12-25

okmttdhr added a commit to okmttdhr/Miss-YT that referenced this issue Feb 7, 2017
fix `Please provide additional annotation(s) to determine whether case 2 works ava`
related issue: avajs/ava#1114
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
Development

Successfully merging a pull request may close this issue.

6 participants