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

Improve flow types #1

Merged
merged 5 commits into from Sep 29, 2019
Merged

Conversation

nicolo-ribaudo
Copy link
Contributor

  1. Generator<...> is the return type of a generator function, not the generator itself

  2. Add an Args type parameter, so that it's possible to specify arguments type in a way
    that works both inside and outside the gensync call:

    var fn = gensync<[number, number], string>(function* (x, y) { return "" });

I had to use any because with mixed I got errors about it not being compatible with the types of the generator function.

1. `Generator<...>` is the return type of a generator function, not the generator itself
2. Add an `Args` type parameter, so that it's possible to specify arguments type in a way
   that works both inside and outside the `gensync` call:

   ```js
   var fn = gensync<[number, number], string>(function* (x, y) { return "" });
   ```
@nicolo-ribaudo
Copy link
Contributor Author

Ok, I didn't understand the Generator part correctly. The bug was in the module.exports type annotation, not in Handler

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Sep 27, 2019

Note that this typings don't play well with variadic functions, since a tuple has a fixed length, but except for a few cases are working well for @babel/core.

} & (
| { async(...args: Args): Promise<Return> }
// ...args: [...Args, Callback]
| { errback(...args: any[]): void }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically these are both optional too. Does | {} also work here? I have no idea if Flow can handle that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment it uses the sync function if neither are provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| {} doesn't work because it isn't an exact object, and if it was we couldn't use &.

@loganfsmyth loganfsmyth merged commit 0716779 into loganfsmyth:master Sep 29, 2019
@nicolo-ribaudo nicolo-ribaudo deleted the patch-1 branch September 29, 2019 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants