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 TypeScript type inference #374

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Improve TypeScript type inference #374

merged 1 commit into from
Sep 26, 2019

Conversation

Avaq
Copy link
Member

@Avaq Avaq commented Sep 25, 2019

After the discussions had in #337 and #370, I think our problems cannot be solved without compromising the strictness of the chain and chainRej functions. This PR makes the following changes to get type inference to work well:

/cc @edgarjrg, @tetsuo

- Revert #230 and reintroduce the usage of the 'never' type
- Relax the signature of chain and chainRej to allow TypeScript to
  derive types when merging a Future of a never with a Future of a
  real type. Closes #337 and closes #370.
- Move generic types closer to where they are used in the function
  signature. Closes #372 and closes #373 through supersession.

Co-authored-by: Edgar Rodriguez <edgarj.rodriguezg@gmail.com>
@Avaq
Copy link
Member Author

Avaq commented Sep 25, 2019

@davidchambers I asked you for a review because I know you've been working on typings for Sanctuary, and you would've likely encountered the same issues with chain for Either.

@codecov

This comment has been minimized.

@Avaq
Copy link
Member Author

Avaq commented Sep 25, 2019

Note that the changes I made to chain and chainRej is this PR go against things I have said in the past (see below). The reason I am doing this now, is because it seems to be the only way to get TypeScript to infer the types properly.

@Avaq Avaq changed the title Improve TypeScript typings Improve TypeScript type inference Sep 25, 2019
@tetsuo

This comment has been minimized.

Copy link
Contributor

@davidchambers davidchambers left a comment

Choose a reason for hiding this comment

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

Allowing TypeScript users to use untagged unions seems reasonable to me. I avoid them, but they are not inherently problematic in TypeScript.

Move generic types closer to where they are used in the function signature.

This is undoubtedly a good change.

@Avaq
Copy link
Member Author

Avaq commented Sep 26, 2019

Together with some regular TypeScript users I have tried to find a solution which does not compromise on the strictness of chain, but have found only solutions whose compromises are worse. I am going to merge this PR.

@Avaq Avaq merged commit 0a751fe into master Sep 26, 2019
@Avaq Avaq deleted the avaq/typings branch September 26, 2019 14:02
Avaq added a commit that referenced this pull request Oct 8, 2019
Breaking changes

- #345 #383 The modular version of Fluture has been moved from `index.mjs` to
  'index.js', and has been made compatible with Node 12.
  In Node 9, 10, and 11, the modular version must now be loaded with the help
  of the 'esm' loader.
- #378 The 'fold' function has been renamed to 'coalesce'.
- #377 The 'Future' constructor is no longer overloaded with a version that
  allows the computation to return 'void'.

New features

- #355 The npm package now ships with some of Fluture's testing internals.
  The API is experimental and subject to breaking changes.

Improvements

- #374 TypeScript types have been improved to get better inference.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants