Given Dart's new focus on being the _best_ client/UI language, I thought I'd revisit this.
Internally there is a long-ish thread with customers having issues, primarily around Future and async/await, and UI components that are shorter-lived than the pending operations encapsulated by the Future.
An internal user writes:
With async/await it is very easy to forget that this is an asynchronous code, and the state of the object can change in the background. I think we cannot have a "synchronized" keyword like in Java, as this code is expected to be "interrupted", but I was wondering if we can have some other language support for that. Any ideas? Or maybe there are some best practices on how to avoid such issues?
/cc @lrhn who already jumped into this conversation, and pointed out issues with it :)
This seems to affect both AngularDart and Flutter, though in different ways.
A basic pattern is something like below:
class Comp implements OnDestroy, OnInit {
final State state;
Comp(this.state);
@override
void ngOnInit() async {
var result = await someRpc();
// You might hit this line after ngOnDestroy has already been called
// and the component is in a dead/inactive/cleaned-up state.
state.update(result);
}
@override
void ngOnDestroy() {
// This is the only lifecycle event you have available to do clean-ups.
state.dispose();
}
}
See the bug? state.update(result) might happen after state.dispose().
Users need to either:
async/awaitUse something like [CanceleableOperation][] from pacakge:async:
var result1 = await new CancelableOperation.fromFuture(someThing(), onComponentDestroyed).valueOrCancellation;
var result2 = await new CancelableOperation.fromFuture(someThing(), onComponentDestroyed).valueOrCancellation;
var result3 = await new CancelableOperation.fromFuture(someThing(), onComponentDestroyed).valueOrCancellation;
ReactJS also has/had this problem, and suggested wrapping all ES6 promises with makeCancelable - fairly close to the CancelableOpreation code above (though less wordy):
const cancelablePromise = makeCancelable(
new Promise(r => component.setState({...}))
);
cancelablePromise
.promise
.then(() => console.log('resolved'))
.catch((reason) => console.log('isCanceled', reason.isCanceled));
cancelablePromise.cancel(); // Cancel the promise
const makeCancelable = (promise) => {
let hasCanceled_ = false;
const wrappedPromise = new Promise((resolve, reject) => {
promise.then(
val => hasCanceled_ ? reject({isCanceled: true}) : resolve(val),
error => hasCanceled_ ? reject({isCanceled: true}) : reject(error)
);
});
return {
promise: wrappedPromise,
cancel() {
hasCanceled_ = true;
},
};
};
Flutter tries to encapsulate many of the issues with FutureBuilder. From what I've hear from both framework authors and customers, there is several holes with this approach, as well. I'll let them fill in the details.
From a strictly AngularDart perspective, we could introduce a class or mixin that tries to encapsulate the hard parts of doing async work in a component:
// Better name pending.
abstract class AsyncComponent implements OnDestroy {
/// Wraps a [future] to ensure it never completes if the component has been destroyed.
Future<T> whenActive<T>(Future<T> future);
}
Example use:
class Comp extends AsyncComponent implements OnInit {
@override
void ngOnInit() async {
final result = await whenActive(someRpcService());
// Will never make it here if the component has been destroyed.
}
}
I imagine we'd have to write some sort of lint to make this very effective, but it is an idea.
For Flutter: I, and I believe the rest of my team, have been happy with the FutureBuilder and StreamBuilder approach but there's a major problem issue with it that prevents us from using it in many places: #33024.
Flutter has even built a custom future-like class to handle that issue with Futures. https://master-docs-flutter-io.firebaseapp.com/flutter/foundation/SynchronousFuture-class.html
I think the underlying issue is very different, but related - Future isn't meant to have synchronous access to data, but at least some users/framework authors have found that inconvenient or difficult to work around.
One more thing I probably should mention is there still an issue of wasting resources, even if we use the wrapper approach or FutureBuilder, there is _no_ way to cancel the underlying RPC.
Imagine downloading a 50Mb video from the server, and the user navigates away, on mobile...
Flutter has to work around some of the same issues regarding async work that Angular dart does. For starters, we have State.isMounted which works exactly as it did in React (before it was deprecated). We do specifically disallow async in lifecycle methods as well, but this is more for general correctness and timing issues with setState. Because Widget trees are immutable, there is less that can go wrong when async work continues past disposal, except for calling setState which we catch with an development error. Of course, the user will still end up doing extra work, and we'll still have to hold more widgets in memory then we would like
class MyWidgetState extends State<MyWidget> {
String _name;
Future<void> doSomeWork() async {
final String name = await Api.getName();
// might have been dismounted, check before calling `setState`
if (isMounted) {
setState(() { _name = name });
}
}
@override
void dispose() {
// only chance to clean up resources.
super.dispose();
}
Widget build(BuildContext context) => ...
}
There are some tangentially related issues with a widget called FutureBuilder. This takes a Future and a builder closure and rebuilds whenever the Future changes. However, recently I've been seeing several issues where it is used incorrectly, but in a non-obvious way. In the example below, instead of passing a Future that is cached on a State object, I just directly pass the invocation of an async method to my FutureBuilder. This works fine until maybe I add an animation on the ancestor of this widget, and now every second when build is called, a new Future is created and the FutureBuilder restarts. Inside the FutureBuilder, it's not possible to tell the difference between any two futures so we _have_ to .
Future<String> _getName() async { ... }
Widget build(BuildContext context) {
new FutureBuilder<String>(
future: _calculation(), // a Future<String> or null
builder: (BuildContext context, AsyncSnapshot<String> snapshot) {
switch (snapshot.connectionState) {
case ConnectionState.none: return new Text('Press button to start');
case ConnectionState.waiting: return new Text('Awaiting result...');
default:
if (snapshot.hasError)
return new Text('Error: ${snapshot.error}');
else
return new Text('Result: ${snapshot.data}');
}
},
)
}
A future has value-like semantics, in that it is immutable and un-cancellable. But it is also an eager computation, and naturally full of side effects. And the combination of these two features means that we can't, as framework authors, solve either problem above. Introducing our own classes like SynchronousFuture is only a stop-gap measure for ourselves, since our users will naturally be drawn towards the simplicity of await.
Good point, one of the issues is that the "best" syntax (await) has a specific contract already.
As someone who is making an application that may have impatient users, having a way to handle this would be great. How would one work around this until a fix comes out?
It's true that Future sets itself between two chairs. It's immutable and sharable (you can listen to it more than once, and passing it around is fine), but it also does represent the result of a computation, including potential throws, and we have issues with passing error futures around (suddenly everybody needs to catch the error).
If we designed Future today, I'd probably have made it one-listen-only. Alas, we didn't, and changing that now is potentially very breaking.
We have CancelableOperation instead of CancelableFuture pretty much on my insistance. The latter would just add a method to Future that nobody else is prepared for. Most futures aren't cancelable, all future operations would throw away the cancelability, so it was really just a hack to pass a "future + cancel function" pair through a type-check that expecter a Future, and cast it back at the other end. Which is a great hack when really necessary, but not good library design.
The issue here seems to be a lack of easy state management and sequencing. There is nothing inherently wrong with the original example except that the setState is called without checking that it's still valid. Everything up to that was fine. It was not wrong to call ngDestroy before ngOnInit was done, even if that means the component was live without being fully initialized (I
guess it was not wrong ... if it was, then ngOnInit must just not be async at all). So, what we need is not mutual exclusion, that would just have prevented you from calling ngDestroy.
The whenActive wrapper will work (should probably make it state.whenActive), but only if you remember using it, and then you could also just write if (state.isDisposed) return; after the await.
Is the answer here perhaps for state.update to throw when the state has been disposed?
Throwing has issues, but if it throws a CancelledException and the caller of ngOnInit() captures such from any returned future, then it might be enough.
I think that's the only way to ensure that the issue is addressed. Everything else is adding more code to ensure something, but nothing prevents you from just forgetting that extra code.
There is still some room for language fiddling around this, including await, even if I don't think it will fix this particular problem.
If we ever get automatic resource disposal (like C#'s IDisposable and using), then a cancelable operation should be disposable. We'd probably use cancel as the dispose method name then (too many things already use that). Then CancelableOperation could be disposable. Wouldn't make a difference here, though, since the cancel needs to happen in a separate method.
We could also make other objects than Future "awaitable". Maybe just anything with a Future-like then method, but probably we have to add something new that Future also gets then. Then CancelableOperation could be both awaitable and disposable. Still not sure if that would actually help.
No plans to fix this in the platform libraries. It's a domain specific issue with state management.
Most helpful comment
From a strictly AngularDart perspective, we could introduce a class or mixin that tries to encapsulate the hard parts of doing async work in a component:
Example use:
I imagine we'd have to write some sort of lint to make this very effective, but it is an idea.