-
Notifications
You must be signed in to change notification settings - Fork 210
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
Add "asyncOr", similar to "async", for FutureOr #2033
Comments
Note that this doesn't have to return a The return type can change, as long as:
|
What other return types would you have in mind?
Can you clarify what you mean by this?
What makes I think it could be useful to allow a return type of |
A custom type that would be made for the occasion, likely a subclass of So: MaybeSynchronousFuture<T> fn() asyncOr {
}
It's about existing APIs that rely on
This prevents making lint-rules such as "consider using |
Imo What it seems to me like you really want here is a better way of easily dealing with That could come in the form of some new type of async function, or possibly just modifications to the existing spec, something like:
Ya this is exactly why I would want to be able to get the value of potentially synchronous |
I wholeheartedly agree. But my understanding is that not all members of the Dart/Flutter team agree, hence why I mentioned using an alternative |
And I am one of those who disagree. 😄 There is no idiomatic way to return a potentially synchronous/potentially asynchronous result because you should never do that. Any code which receivers something typed as Providing a way to have the same code act as both synchronous and asynchronous is not just not a goal, it's something I'd try to actively discourage. So, back to the original proposal: It's not impossible. It'd be like an I just don't think it's a good idea. Make two functions instead, one which allows |
It is inherently potentially asynchronous which is a very different thing.
This just isn't always practical though. In performance critical code that has to deal with potentially async things, you really want to avoid thrashing the event loop. In fact the entire existance of A common example of this is a cached async operation. You have to deal with the fact that it might need to do some async work, but usually the value is cached. And you may read this value repeatedly and often, and want that operation to continue synchronously if the cached value is available. As long as the API statically signals (via
The problem is you want the function that accepts a |
That's not why That's why every platform API or language feature which recognizes a If you create an API which is sometimes synchronous and sometimes not, it won't interact well with the platform library. You'll have to split the code paths before calling platform code, otherwise you end up with a definitely asynchronous result. (I'll admit that I don't know what "thrashing the event loop" means. It does sound bad.) |
The issue with being against code that can be both synchronous and asynchronous is, there are legitimate use cases. Splitting the code into two function, one sync and one async, is not something reasonable. In many cases this distinction is needed when some of the computation is cached. It is the same code being invoked, but we don't need to load an asset or do a network request because that was done before. Forcing the code to be async here would cause a flicker on the UI. Which is where Flutter instead relies on SynchronousFuture That's an important problem. We can't simply dismiss it with "you should never do that", because that's simply not true |
The existence of
The problem is that if you don't know whether something will provide its value synchronously or asynchronously, you still risk flickering the UI. So, presumably you are seeding the value in a way which ensures that it's going to be synchronous when it matters. That's a perfectly reasonable design, because you know whether it's synchronous or not. It's just trying to fit this into a design which wasn't made for that. The direct approach to the problem would be a class like: import "package:async/async.dart";
class AsyncCache<T> {
final Future<T> computation;
Result? _result;
AsyncCache(this.computation) {
Result.capture(computation).then((result) {
_result = result;
});
}
bool get hasResult => _result != null;
T get result {
var result = _result;
if (result == null) throw StateError("No value");
ValueResult? value = result.asValue;
if (value != null) return value.value;
ErrorResult error = result.asError!;
Error.throwWithStackTrace(error.error, error.stackTrace);
}
} That'll allow you to access a future's result synchronously ( It doesn't try to shoehorn the two approaches into the same code path (which then has to get at least twice as complex, and requires a type check - one which 95% of people will get wrong for a I might be more worried about the complexity we'll see people write themselves into in order to make potentially synchronous computations, than about the feature itself. I'd be totally down for making It has some consequences: We currently complete the returned future asynchronously in returns that happen during the initial synchronous part of an If we say that an Or, we could introduce a different function body marker, like My main worry is then why should every Returning a If it's easy to misuse, I don't think it's a good language feature. I fear this feature, which is otherwise quite technically possible, |
Ya this would be I think the best solution, combined with making
That is a good case for a special keyword, I hadn't considered that. Having both options seems a bit weird though.
It is only really a cost on the receiver because we don't have the feature being asked for here. These features are really all about making it easier to handle
I would hypothesize that functions which accept futures as parameters are relatively rare compared to ones that return futures. They certainly exist (
I guess I am not quite understanding exactly what the concerns are? Are you concerned about the cost of the type checks, or something else? |
I don't quite see what can be easy to misuse here either. I could see the problem if we were to make |
The infinite loop is a valid worry. Say, you call an async method until some variable changes, and that async method becomes asyncOr and starts caching results for up to five seconds instead of repeatedly querying the backend. And it returns the cached result synchronously. Then you will busy-wait for five seconds and block all other execution, including the one which would change the variable you're waiting for. var oldValue = variable;
do {
await checkServerUp(); // Used to give other code time to run, not anymore.
} while (oldValue == variable); The misuse I'm worried about is people making functions be Such authors will likely think that they are helping their users, with a comment like "// asyncOr for speed!". What's not to like is the cases where you don't And it possibly turns off the When I look at a feature, I try to look at the incentives to use or not use that feature. Who benefits? Who does it cost? Here, I fear the feature might be used too much, because the incentives are there to change I don't expect users to make rational and well thought out choices about every little thing in their program. They'll learn some rules-of-thumb and "best practices" and apply them blindly. If the language is well structured and designed, that will usually work. That's what this design encourages, and I don't think it's a good outcome. If we think it is a good, or reasonable, outcome, and that we want to fundamentally change the Dart That's a valid design choice for Dart, if done properly. That's my worry. Feel free to disagree. |
Interestingly, I count ~100 existing functions with That also suggests to me where this is coming from. I generally say that a function in your public API should not return These interfaces are input objects to a framework. A method on such an object can be treated like a function argument, an input to the framework code, and as such, allowing It worries me a little that being synchronous is such a big advantage that it's worth worrying about whether the code ends up being synchronous or asynchronous, that suggests that asynchrony is still too expensive. Accepting both is fine, preferring synhronous over asynchronous is at least a little problematic. |
That's something which would be caught in development immediately, and it'd be a matter of adding an We could possibly have a lint for that too. That's something that the analyzer can catch
I'd say that's quite unlikely:
We could (and likely should) have a lint that says "asyncOr unused, the function can never return synchronously" such that we have: FutureOr<T> fn() asyncOr { // warning, http.get returns a Future not FutureOr
return http.get('...');
}
FutureOr<T> fn() asyncOr { // ok
if (cache != null) return cache;
return cache = await http.get('...');
}
That sounds a bit exaggerated to me. To begin with, these are likely fairly rare. And we could solve all of those with one single extension: extension<T> on FutureOr<T> {
Future<T> get asFuture => this is future? this : null;
} which allows
|
Performance impact is not what we're after. The problem at hand is avoiding flickers when the UI tries to render data that is readily available yet locked behind an unnecessary await. So it's a known issue with known workarounds. The case I'm making is that the code needed to solve this problem is not pretty. |
It's not specific to caching data. |
I don't agree FutureOr is not a base class that objects have to implement. Everything is a FutureOr |
What's stopping NetworkImage from returning a dynamic instead Future with SyncchronousFuture sometimes ? The base class interfaces the method I agree it would be easier to await on the other side if it's maybe a future though wouldn't the other proposal to remove the await from async methods solve this ? |
Well, at that point, it might as well just be a keyword. I don't think special-casing a feature that goes this deep into the mechanics and philosophies of asynchrony is a good idea. If it impacts a language mechanic, it should be a language feature. It seems that the discussion has otherwise come to the consensus of modifying the behavior of
And from @lrhn (#2033 (comment)):
Seems to me like you two agree on the approach. |
Right, I agree with you. That's why I quoted -- twice -- that it would be better to change the existing mechanics than to introduce a whole new keyword that does basically the same thing. |
To come back to this, personally I think a real problem with this issue is, packages cannot fix it themselves. That's in part why I wanted statement-level macros for metaprogramming, so that packages could implement their own await-like keywords if they wanted to. After-all there are numerous use-cases for custom await-like keywords, such as:
|
That won't work AFAIK. Unless a new proposal has been made with different rules, macros are not able to modify user-defined code. |
But that would effectively break breakpoints I believe. My understanding is that the augment proposal relies on a new "super" feature, where generated code can call @macro
void fn() {
print('hello');
}
// generated augment library
void fn() {
print('before');
super();
print('after');
} We could theoretically not use // generated augment library
void fn() {
print('before');
print('hello');
print('after');
} but I believe that |
Do you have a link to that thread? I don't know which one you're referring to |
Oh, you're referring to Considering the amount of pushback I got when suggesting various solutions to this issue (sourcemaps, expression macro, ..), I'm almost certain that your proposal would indeed break breakpoints and that would be considered as "expected behavior" |
That's a marginally different use-case. If tomorrow @async
Future<int> fn() {
int result = @await fetch(...);
int result2 = @await fetch2(...);
return result + result2;
}
// augment
Future<int> fn() {
return Future(() {
return fetch(...).then((result) {
return fetch2(...).then((result) => result + result2);
});
});
} then this would be a significant degradation of the user experience to expect folks to place their breakpoints in the generated code. That's why the JS land with Babel/TS/... uses source-maps. It is a critical piece to have a good developer experience. |
We can't use the So the generated code will be more complex because of that. I'd expect: @asyncOr
FutureOr<int> fn() {
print('hello');
int value = wait(someFutureOrExpression(...));
int value2 = wait(another());
return value * value2;
} to generate something among the lines of: FutureOr<int> fn() {
print('hello');
final _$tmp1 = someFutureOrExpression(...);
FutureOr<int> _$body1(int value) {
final _$tmp2 = another();
FutureOr<int> _$body2(int value2) {
return value * value2;
}
return _$tmp2 is Future<int> ? _$tmp2.then(_$body2) : _$body2(_$tmp2 as int);
}
return _$tmp1 is Future<int> ? _$tmp1.then(_$body1) : _$body1(_$tmp1 as int);
} |
No. Macros cannot change the prototype of a function, so the return type has to stay the same.
Right. But then that's a language change, at which point we might as well implement the entire feature instead of only changing one part of relying on macros for the |
Using build_runner, this trick can only be applied to class methods, not static functions. But like I mentioned before the problem is the developer experience. |
I'm not sure what your proposal solves. We can already use FutureOr<String> getWeatherForecast() {
if (weatherForecast != null && getForecastAgeInSeconds() < 60) {
return weatherForecast;
}
return Future(() async {
weatherForecast = parseResponse(await http.get(... request to Weather Service));
timestamp = DateTime.now();
return weatherForecast;
});
}
methodUsingWeatherForecast() async {
var f = getWeatherForecast();
String forecast = f is Weather ? f : await f;
// use forecast
} |
A common UI problem is to want to make potentially asynchronous code synchronous if possible, to avoid flickers where progress indicators show only for a frame or two.
While there are solutions to this problem, the issue is, those solutions are strictly incompatible with
async
/await
– decreasing the code readability quite a bit.Proposal
To solve the problem of async/await being incompatible with code-paths that are potentially synchronous, we could introduce an
async
variant that returns aFutureOr
Meaning we could write:
With this variant,
await
may execute synchronously (when it currently would always be async). Meaning that the previous snippet would be equivalent to doing:The text was updated successfully, but these errors were encountered: