[go: up one dir, main page]

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

fix(Task.all): fixes Task.all when called with an empty array (closes #5) #6

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

dggluz
Copy link
Contributor
@dggluz dggluz commented Sep 30, 2018

Description

Now Task.all([]), returns a Task resolved with an empty array.

I also added the overload to support the case when the input array is never[] (the typechecker didn't complain before, but could not infer correctly the resulting Task).

Related issues

Closes #5.

@hrajchert
Copy link
Contributor

I'm not sure about the never[] overload... why is this necessary?

@dggluz
Copy link
Contributor Author
dggluz commented Oct 2, 2018

That is because of the most generic typing overload (function all <T, E> (tasks: Array<Task<T, E>>): Task<T[], E>) that was allowing empty arrays but couldn't infer any resolution nor rejection types for them:

task-all-empty-array-error

I think that is better typed with the never[] case.

@dggluz
Copy link
Contributor Author
dggluz commented Oct 2, 2018

I think another option is to default the generics to never in the most general case. It would be:

function all <T = never, E = never> (tasks: Array<Task<T, E>>): Task<T[], E>

I don't have a preference for any of those options.

@hrajchert
Copy link
Contributor

Well, once we upgrade to 3.x we could use the unknown type. Of those two options I would prefer the second one, but even better is not type it.

When you do

let a1 = [1, 2, 3]
a1.push(4);

you don't specify the type because typescript can infer it. But if you initialize with an empty array you normally specify the type beforehand.

let a2: number[] = [];
a2.push(1)

Having an array of never is not useful. If you don't want to get the {} type infered and prefer never or number you can always specify it

Task.all<never>([])

In conclusion, I would remove the never part.

@dggluz dggluz force-pushed the fix-task-all-with-empty-array branch from 0732da1 to a3ec6c6 Compare October 2, 2018 18:00
@dggluz
Copy link
Contributor Author
dggluz commented Oct 2, 2018

Ok, I removed that! I think the PR is ready to be merged!

@hrajchert hrajchert changed the base branch from master to develop October 8, 2018 21:35
@hrajchert hrajchert merged commit 615b8f6 into ts-task:develop Oct 8, 2018
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.

2 participants