Change type of `futures` parameter of `Future.wait` to `Iterable<FutureOr<T>>`

[jakemac53]

Now that we have FutureOr, and more apis are starting to use it, I think it makes sense to take advantage of this in Future.wait if possible (error handling might get weird, not sure).

The main use case is when calling .map on some iterable and calling a function that returns a FutureOr. Today you have to wrap that call in an async method to make sure the type of your iterable is Future<T>, but ideally you wouldn't have to do that.

Ideal:

FutureOr<...> doStuff(input) => ...

Future doLotsOfStuff(Iterable inputs) => Future.wait(inputs.map(doStuff))

Required today (or something similar):

FutureOr<...> doStuff(input) => ...

Future doLotsOfStuff(Iterable<...> inputs) =>
    Future.wait(inputs.map((input) async => doStuff(input)))

[jakemac53]

I don't think that this would be a breaking change, but I could be wrong :)

[lrhn]

I'm not sure I would want that change.
It would allow combining futures and non-future values, and awaiting only the futures, but mixing up types like that is not something we encourage.

In general, I don't plan to make many functions that expect a FutureOr as argument. You are expected to know, at all times, whether you have a future or not, and it's only for methods like Future.then where we would like to have support for both futures and values in the callback return, and we don't want to invent a new name, that FutureOr really makes sense.
You can think of it as a substitute for overloading - we can't have two then methods, so we make one method that handles both cases. The two case for Future.wait would be Iterable<Future<T>> and Iterable<T> - and the latter doesn't need waiting.

[jakemac53]

I don't think this is a huge issue because FutureOr is still used relatively sparingly, but I also don't know of a concrete reason not to allow it. It also seems to be more consistent with the await semantics which allow you to await anything.

It would allow combining futures and non-future values, and awaiting only the futures, but mixing up types like that is not something we encourage.

I agree that generally it's a weird pattern, but if you expose an api that allows users to return a FutureOr<T> then you do have to deal with this. This is exactly how I noticed it, I updated an api that users implement to return a FutureOr<String> instead of a Future<String> in source_gen.

In general, I don't plan to make many functions that expect a FutureOr as argument. You are expected to know, at all times, whether you have a future or not, and it's only for methods like Future.then where we would like to have support for both futures and values in the callback return, and we don't want to invent a new name, that FutureOr really makes sense.

Imo, this is just forcing extra burden on the developer for a likely increasingly common pattern in framework/library code. I see essentially three options when running into this issue as a developer:

1. Future.wait(myList.map((v) async => v));
2. Future.wait(myList.where((v) => v is Future));
3. Future.wait(myList.map((v) => v is Future ? v : new Future.value(v)));

Option 1 adds extra futures but is the least boilerplate, option 2 is possibly more efficient but if you care about having a final list of all the synchronous values (in the original order) from myList then it becomes more difficult. Option 3 is better but has the most boilerplate, and still you end up creating extra futures for really no good reason. If Future.wait handled it directly it could skip creating futures for the synchronous values completely, so it can be more efficient than anything I can do in user code (that I can think of at least).

I don't think that people are going to start wrapping a bunch of iterables of known synchronous values in a Future.wait in any case.

You can think of it as a substitute for overloading - we can't have two then methods, so we make one method that handles both cases. The two case for Future.wait would be Iterable<Future> and Iterable - and the latter doesn't need waiting.

The source_gen example above, or any time you are inserting return values from a method that returns a FutureOr into a list may result in mixed values in the same list, so its not really the same as overloading imo, it's really a union type.

[lrhn]

This is caused by having a function with a FutureOr return-type.
If you change that function to always returning a Future, then your problem goes away, and I really do recommend doing that for public APIs.
If it's an internal function, or something passed in through the public API like here, then you are the person in position to handle the situation (and probably the one who got yourself into the situation by accepting something that returns a FutureOr to begin with).
I'd just do the map with (x)=>new Future.value(x) (which is why we want constructor tear-offs), and if I haven't made the return a future (of the correct type) directly yet, I should.

Functions that return FutureOr should not proliferate. We shouldn't make every function that accepts a Future now accept a FutureOr just because we can, in many cases the functionality on the non-future should be fully synchronous and not embedded in an async function.
So, functions returning FutureOr should be exceptions, and something you handle internally in a class or library if you expose an API that allows users to pass in either, and you should do that judiciously - that is, only when you expect users to often be in a situation where they have the desired value as a non-future.

That said, I can see that that internal handling can use some general tools to go from FutureOr to either Future (new Future.value) or value (await) and a way to generalize that to multiple FutureOrs in various ways (parallel, sequential, fail-early/late, etc), so basically what Future.wait does.
I still don't want it to be Future.wait, and I don't think we can do FutureOr.wait since FutureOr is really a synthetic type, not a real class. So maybe something in package:async would be better.

[natebosch]

FWIW these are signatures that consumers of our package implement. When the return type is Future since we know some authors will need asynchronous work we get asked "Why do I have to return a Future when my implementation is synchronous?"

[jakemac53]

This is caused by having a function with a FutureOr return-type.
If you change that function to always returning a Future, then your problem goes away, and I really do recommend doing that for public APIs.

Forcing all users to wrap their code in a Future just to make my life easier doesn't seem like a good solution to me. FutureOr can be very useful because it enables async code with forcing it. Unless their is a concrete reason (performance, other?) not to use it then I personally would like to see it used more in libraries/frameworks, not less.

I'd just do the map with (x)=>new Future.value(x) (which is why we want constructor tear-offs), and if I haven't made the return a future (of the correct type) directly yet, I should.

Yes, constructor tear-offs would be great here :).

Functions that return FutureOr should not proliferate.

Is the reasoning for this that we don't want end users to have to deal with them? Imo that isn't so much of an issue if all the core libs support them, you shouldn't see a lot of type checks for a Future vs value in that case. Future.wait is one example where the core libs don't make it automatic today, I don't know if there are a lot of others. The await keyword already makes handling this easy for the non-iterable case.

We shouldn't make every function that accepts a Future now accept a FutureOr just because we can, in many cases the functionality on the non-future should be fully synchronous and not embedded in an async function.

Agreed, but imo we should use it wherever it adds value (in the form of convenience in this case) and doesn't leak back to the user. For instance Future.wait would still always return a Future, not a FutureOr. This would just be a convenience measure and can be handled completely internally without repercussions for users.

So, functions returning FutureOr should be exceptions, and something you handle internally in a class or library if you expose an API that allows users to pass in either, and you should do that judiciously - that is, only when you expect users to often be in a situation where they have the desired value as a non-future.

I think that Future.wait fits within this description pretty well. It might not be that common of a use case, but enabling it doesn't compromise the more general case either.

That said, I can see that that internal handling can use some general tools to go from FutureOr to either Future (new Future.value) or value (await) and a way to generalize that to multiple FutureOrs in various ways (parallel, sequential, fail-early/late, etc), so basically what Future.wait does.
I still don't want it to be Future.wait, and I don't think we can do FutureOr.wait since FutureOr is really a synthetic type, not a real class. So maybe something in package:async would be better.

I don't know how discoverable it would be in package:async or whether people would want to take a dependency on it just for this. It would give us somewhere to point users though, and it would also give us a path towards testing some of these things out and see if they become popular enough to justify editing the core libs or not.

[lrhn]

Functions that return FutureOr should not proliferate.
Is the reasoning for this that we don't want end users to have to deal with them?

Yes. A function returning FutureOr is strictly harder to use than one returning Future.
The user of the function must be able to handle both a Future result and a non-Future result, and needs to do extra checking to separate the two.
That means that the code is already able to handle the Future, so always returning a Future is just simpler. You are not helping the user by returning a FutureOr, only yourself, and that's not a good API design reason :)

Imo that isn't so much of an issue if all the core libs support them, you shouldn't see a lot of type checks for a Future vs value in that case. Future.wait is one example where the core libs don't make it automatic today, I don't know if there are a lot of others. The await keyword already makes handling this easy for the non-iterable case.

True, and it's not completely unreasonable to change Future.wait, but it is adding extra complication that affects every user for something that can be adequately handled by just converting all values to futures in the few cases where you actually have mixed types:

Future.wait(list.map((x) async => x))

In other words: It is not free. Every time we accept FutureOr, we need to add a type-check on the value before we can use it, plus an unpredictable branch to handle the two cases.

There are places where we do accept FutureOr anyway, mainly for historical reasons, and convenience when interacting with those historical places. If we hadn't had Future.then already accept both in Dart 1, we probably wouldn't have added FutureOr for strong mode. And there are a few of those places I would like to retract for Dart 2 because it's not really necessary.

[jakemac53]

True, and it's not completely unreasonable to change Future.wait, but it is adding extra complication that affects every user for something that can be adequately handled by just converting all values to futures in the few cases where you actually have mixed types:

Future.wait(list.map((x) async => x))
In other words: It is not free. Every time we accept FutureOr, we need to add a type-check on the value before we can use it, plus an unpredictable branch to handle the two cases.

Agreed that the type checks would be unfortunate. I think with strong mode you can make this cheaper in the general case by doing a single type check on the argument for is Iterable<Future>, and then skip the individual type checks on each item if that is true which should be quite cheap for that case. This type of check doesn't work in Dart 1 though.

[natebosch]

Just to clarify:

Yes. A function returning FutureOr is strictly harder to use than one returning Future.
The user of the function must be able to handle both a Future result and a non-Future result, and needs to do extra checking to separate the two.
That means that the code is already able to handle the Future, so always returning a Future is just simpler. You are not helping the user by returning a FutureOr, only yourself, and that's not a good API design reason :)

The places we've been doing this are places where we allow the user to return the FutureOr and we take on the additional complexity. We have not been changing our return types to FutureOr or expecting the user to deal with the additional complexity.

[lrhn]

The places we've been doing this are places where we allow the user to return the FutureOr and we take on the additional complexity. We have not been changing our return types to FutureOr or expecting the user to deal with the additional complexity.

Then you are using it correctly :)
Even for that case, I wouldn't change all places that accept a Future (any Future in contravariant position in the public API) to be a FutureOr. It's OK to just ask for a Future, users can convert using Future.value if needed. It's only for cases where the users is expected to often have a value instead of a future that I would make the change. That is, the extra complexity, even if it's only on yourself, should be worth it.

For these correct usages, you do need some helper methods, but moving them into package:async would allow the helpers to use the extra functionality of that package, like the Result class which makes generalized async code much simpler to work with. Perhaps you could use functions like:

class Result... 
  ...
  /// Waits for all futures among [computations] and collects their results.
  static Future<List<Result<T>>> captureAll<T>(Iterable<FutureOr<T>> computations);
  /// If all [results] are values, returns a value result of a list with those values.
  ///
  /// If any of [results] is an error, returns an error result with the first error,
  /// or if [collectErrors] is true, with a list of all the errors as [MultiError].
  static Result<List<T>> flatten(Iterable<Result<T>> results, {bool collectErrors: false})
}

That would allow you to capture all the futures and collect the values, and then you can do the error handling/clean-up manually instead of the wait function having to handle it for you. We can't do that in the dart:async library because it doesn't have the Result class available. (Should it?)

(I'd want a better name for captureAll that suggests that it also handles values, so captureAll can be used for Iterable<Future<T>>, suggestions welcome :)