Skip to content

Make typescript types generic#81

Merged
cowboyd merged 2 commits intomasterfrom
generic-types
Mar 9, 2020
Merged

Make typescript types generic#81
cowboyd merged 2 commits intomasterfrom
generic-types

Conversation

@jnicklas
Copy link
Copy Markdown
Collaborator

@jnicklas jnicklas commented Mar 5, 2020

This way the result of an operation can be carried forward, which makes typechecking better. Adding these generic types doesn't make a huge difference, but at least it complains when we get the return value of a generator function wrong, which is nice.

@jnicklas jnicklas requested a review from cowboyd March 5, 2020 14:17
Comment thread index.d.ts
// TypeScript Version: 3.7
declare module "effection" {
export type Operation = OperationFn | Sequence | Promise<any> | Controller | undefined;
export type Operation<T = any> = undefined | OperationFn<T> | Sequence<T> | PromiseLike<T> | Controller<T>;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I've defaulted these types to any, to maximize compatibility with existing code.

@frontsidejack
Copy link
Copy Markdown
Member

A preview package of this pull request has been released to NPM with the tag generic-types.
You can try it out by running the following command:

$ npm install effection@generic-types

or by updating your package.json to:

{
  "effection": "generic-types"
}

Once the branch associated with this tag is deleted (usually once the PR is merged or closed), it will no longer be available. However, it currently references effection@0.5.1-5f59183 which will be available to install forever.

Generated by 🚫 dangerJS against 105987e

This way the result of an operation can be carried forward, which makes typechecking better, since we can infer the type of the variable that invokes the yield.
@frontsidejack
Copy link
Copy Markdown
Member

A preview package of this pull request has been released to NPM with the tag generic-types.
You can try it out by running the following command:

$ npm install effection@generic-types

or by updating your package.json to:

{
  "effection": "generic-types"
}

Once the branch associated with this tag is deleted (usually once the PR is merged or closed), it will no longer be available. However, it currently references effection@0.5.1-c4bcde1 which will be available to install forever.

Generated by 🚫 dangerJS against fefe508

Copy link
Copy Markdown
Member

@cowboyd cowboyd left a comment

Choose a reason for hiding this comment

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

Looks good, although I do think that we need to change the type of timeout. Also, what is the tradeoff of using unknown instead of any. The reason I say is that it might help us get to noImplicitAnysville sooner

Comment thread index.d.ts Outdated
@frontsidejack
Copy link
Copy Markdown
Member

A preview package of this pull request has been released to NPM with the tag generic-types.
You can try it out by running the following command:

$ npm install effection@generic-types

or by updating your package.json to:

{
  "effection": "generic-types"
}

Once the branch associated with this tag is deleted (usually once the PR is merged or closed), it will no longer be available. However, it currently references effection@0.5.1-d5d18e3 which will be available to install forever.

Generated by 🚫 dangerJS against 8c3bcdd

@cowboyd cowboyd merged commit e042c5f into master Mar 9, 2020
@cowboyd cowboyd deleted the generic-types branch March 9, 2020 14:40
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.

3 participants