Skip to content

[change] loadLibrary should return FutureOr #53410

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

Closed
gmpassos opened this issue Sep 1, 2023 · 11 comments
Closed

[change] loadLibrary should return FutureOr #53410

gmpassos opened this issue Sep 1, 2023 · 11 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@gmpassos
Copy link
Contributor

gmpassos commented Sep 1, 2023

Change Intent

The loadLibrary, used to ensure that a deferred library is load, should return a FutureOr for optimization purpose.

Justification

If loadLibrary returns a FutureOr will be possible to avoid waiting of a Future already resolved.

Here is a current workaround that avoids await to call a code that depends on a deferred library:

// The business part of the UI should be lazy loaded:
import 'ui_business.dart' deferred as ui_business;

bool _ui_business_loaded = false;

/// Perform a `call` that depends on `ui_business`.
FutureOr<R> ui_business_call<R>(R Function() call) {
  if (_ui_business_loaded) {
    return call();
  }

  return _ui_business_call_async<R>(call);
}

Future<R> _ui_business_call_async<R>(R Function() call) async {
  await ui_business.loadLibrary();
  _ui_business_loaded = true;
  return call();
}

With the proposed change:

// The business part of the UI should be lazy loaded:
import 'ui_business.dart' deferred as ui_business;


/// Perform a `call` that depends on `ui_business`.
FutureOr<R> ui_business_call<R>(R Function() call) {
  var load = ui_business.loadLibrary();
  if (load is Future) {
    return load.then((_) => call());
  } else {
    return call();
  }
}

Impact

Current Dart code won't be affected if using await.

Only a code using then will need to be changed.

Mitigation

FutureOr.then could also be provided by dart:async, improving usage of FutureOr.

If FutureOr.then is also provided, no change in current code will be needed.

Change Timeline

No deprecation is needed.

Associated CLs

None

@gmpassos gmpassos added the breaking-change-request This tracks requests for feedback on breaking changes label Sep 1, 2023
@github-project-automation github-project-automation bot moved this to In review in Breaking Changes Sep 1, 2023
@kevmoo
Copy link
Member

kevmoo commented Sep 1, 2023

SGTM! I'm not (too) worried about .then on Future in this context. Maybe in some old code.

@rakudrama
Copy link
Member

@gmpassos Could you explain the practical impact of the optimization? await on a completed Future for the second time the await happens should be fast, even though it permits other code in the microtask queue to run. How much gain do you see from the optimization?

Implicit in this request is that loadLibrary() actually returns something other than a Future when the import has been successfully loaded.

@rakudrama
Copy link
Member

Note that it is possible to generalize the work-around to one piece of code that works for all deferred imports.
The trick is to tear-off the loadLibrary method and use it as a key into a Set of loaded libraries.

import 'dart:async';

import 'dart:typed_data' deferred as typed_data;

typedef LoadFunction = Future<void> Function();

FutureOr<R> callWithLoadedLibrary<R>(LoadFunction loader, R Function() action) {
  print('callWithLoadedLibrary');
  if (_loaded.contains(loader)) return action();
  print('loading');
  return loader().then((_) {
    print('loaded');
    _loaded.add(loader);
    return action();
  });
}

final Set<LoadFunction> _loaded = {};

main() async {
  final r1 = await callWithLoadedLibrary(typed_data.loadLibrary, () {
    print('f1');
    return typed_data.Uint8List(3);
  });
  
  final f2 = callWithLoadedLibrary(typed_data.loadLibrary, () {
    print('f2');
    return typed_data.Uint8List(5);
  });

  if (f2 is Future) throw 'unexpected';
  print('done $r1 $f2');
}
}

prints:

callWithLoadedLibrary
loading
loaded
f1
callWithLoadedLibrary
f2
done [0, 0, 0] [0, 0, 0, 0, 0]

/cc @lrhn

@lrhn
Copy link
Member

lrhn commented Sep 1, 2023

I'm generally opposed to giving FutureOr to people.

Either you don't know whether it's done, and therefore you need to have the async code path anyway, on top of the synchronous "optimized" part, or you do know that it's done, because someone checked already, but haven't cached it's result.

Having two interleaved code paths, one sync and one async, doing the same computation, is unlikely to be worth the code size increase. And complexity. And testing trouble, to make sure all the combinations are tested.

If it is worth it, it's because the code is running very often, and then it's better to have a single wait guarding access to everything, and all synchronous code behind that one wait. That's what the example code here does: Do an async operation once, then cache that it was done.

Using multiple functions returning FutureOr for code that is mostly synchronous, except the first time, is just not a design I want to encourage, or support.

@lrhn lrhn added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). and removed breaking-change-request This tracks requests for feedback on breaking changes labels Sep 2, 2023
@gmpassos
Copy link
Contributor Author

gmpassos commented Sep 2, 2023

First and foremost, thank you for the quick response and for participating in this discussion.

Here's a pratical example:

https://github.com/gmpassos/deferred_lib_issue

(The issue only happens when running from dart test: dart-lang/test#2088)


In this example, you can observe how to create a straightforward framework that accommodates the use of both Future and FutureOr, depending on the desired ViewBase implementation.

Just run the web/index.html through webdev:

webdev serve --launch-in-chrome -r

I believe the primary focus of our discussion should not solely revolve around performance but should also emphasize ease of use. I recommend attempting to write code that minimizes the excessive use of Future. This approach will help illustrate my perspective more clearly.

I make a conscious effort to minimize the usage of Future and await in the code,
as their introduction often necessitates the implementation of numerous other methods as async.

The consequence of having numerous async methods is unfavorable; it results in larger code size, complexities in stack traces, difficulties in debugging, increased memory allocation (as each Future generates a new object for the GC), and impedes compiler optimization.

When working with a deferred library that utilizes a non-asynchronous prefix, it introduces a dependency on prefix.loadLibrary(). This, in turn, introduces hidden asynchronous behavior for each exported element from the deferred library. Therefore, a deferred library is more akin to FutureOr (something that can be either asynchronous or synchronous) than to Future.

Creating an application that truly adheres to import compartmentalization can be challenging. However, using a deferred library with numerous exported elements can also be complex due to the hidden asynchronous flow inherent in the deferred pattern.

One thing I truly appreciate about Dart is that it doesn't impose a specific programming paradigm but rather provides you with the best tools for each use case. In the latest updates to the language, we've seen advancements in both Functional Programming and Object-Oriented capabilities. This is remarkable, especially considering that many languages tend to segregate paradigms rather than unify them.

However, when it comes to discussing Future and FutureOr, it seems like you're advocating for the widespread use of async methods. In my view, we should leverage the advantages of FutureOr whenever possible, without obscuring it.


I am the author of the async_extension package, which facilitates smoother interactions with FutureOr:
https://pub.dev/packages/async_extension

@gmpassos gmpassos closed this as completed Sep 2, 2023
@github-project-automation github-project-automation bot moved this from In review to Complete in Breaking Changes Sep 2, 2023
@gmpassos gmpassos reopened this Sep 2, 2023
@lrhn
Copy link
Member

lrhn commented Sep 2, 2023

Using FutureOr as an approach to having both async and sync behavior in the same function is, as I usually see it used, trying to avoid asynchrony entirely for performance reasons. It ends up almost doubling the complexity, and it often ends up as a local minimum in design, instead of taking a larger refactoring step to move all asynchrony up the call stack, to maximize the final synchronous call-trees.

The deferred library is an example of this. You say that it looks like something that could be modelled by FutureOr.
It see it more like an asynchronous factory or initializer function, which you only need to call once.
You shouldn't call it again later, where you would like to be able to use a non-Future return value, if you have already called it successfully once. It's even faster to not call at all.

Using FutureOr mangles that initial step into the entire algorithm, instead of forcing it to be made up-front.

(I'd probably design around a deferred load by creating an object which provides access to the loaded library, and guard creation of that object by waiting on the loadLibrary. Then your code has to be written to pass around that object, which it can only do if the object has been created.)

@gmpassos
Copy link
Contributor Author

gmpassos commented Sep 2, 2023

Here is a code snippet derived from a real application to illustrate the concept:

class MainView {

  FutureOr<R> business_call<R>(R Function() call) {
    // ... handles a deferred library then executes call(), avoiding `Future`
  }

  FutureOr<View> _renderRouteComponent(String? route) {
    switch (route) {
      case 'home':
        return ViewHome(content);
      case 'login':
        return ViewLogin(content);
      case 'register':
        return ViewRegister(content);
      case 'user':
        return ViewUser(content);
      case 'emailVerification':
        return ViewEmailVerification(content);
      case 'network':
        return business_call(() => business.ViewNetwork(content));
      case 'section':
        return business_call(() => business.ViewSection(content));
      case 'item':
        return business_call(() => business.ViewItem(content));
      case 'business_account':
        return business_call(() => business.ViewBusinessAccount(content));
      case 'terms':
        return ViewTerms(content);
      case 'addresses':
        return business_call(() => business.ViewAddresses(content));
      case 'payments':
        return business_call(() => business.ViewPayments(content));
      case 'dashboard':
        return business_call(() => business.ViewDashboard(content));
      case 'financial':
        return business_call(() => business.ViewFinancial(content));
      case 'payment_broker':
        return business_call(() => business.ViewPaymentBroker(content));
      case 'system_integration':
        return business_call(
            () => business.ViewSystemIntegration(content));
      default:
        return ViewHome(content);
    }
  }

}

Before deferring a library, it was a standard switch statement with a regular return for each case. I don't believe that migrating regular code to work with a deferred library should require many changes, such as moving everything to a new class. It would be better for it to remain similar to the natural code (without a deferred library).

I really need to avoid returning a Future when rendering a component, as it would result in double rendering—initially displaying a loading animation until the future completes, and then rendering with the result. By using FutureOr, the rendering becomes instantaneous after the initial deferred load.

The framework expects a FutureOr from a render function, which allows the rendering flow to include Future, especially when a component is loading data, but it does not mandate the use of Future, making development much easier.

Please note that, for now, deferred libraries are only supported by Dart code compiled to JavaScript. We utilize them in a browser environment, which is significantly impacted by performance considerations.

@gmpassos
Copy link
Contributor Author

gmpassos commented Sep 2, 2023

IMHO, dart:async should evolve to facilitate the use of FutureOr for more versatile usage.

@lrhn
Copy link
Member

lrhn commented Sep 4, 2023

Your use of FutureOr here is reasonable.

We generally discourage using FutureOr as a return type (or covariantly) in a public API. That means the API is giving the user either a value or a future, which means the user must be ready to accept getting a Future every time. Such a function is effectively asynchronous. It having an extra synchronous option is an API complication, not a benefit, which is why we don't recommend it for public APIs.

Here, you are providing either a future or a value to pass into a framework API, which is generous enough to accept both. In this case, it even has a visible difference in behavior depending on whether you pass it a value or a future. That's untraditional, but in this use-case its reasonable. The alternative would be to have two APIs, one setValueNow and one setValueAsyncAndShowSpinnerUntilDone, effectively.

Usually when a framework asks for a FutureOr<T> Function(something), they expect to get either a Future<T> Function(something) or T Function(something), and really treat the T Function(something) as a curtesy to so the user can avoid writing (something) => Future.value(realFunction(something)), but they'll treat the result as asynchronous anyway. But you can provide a FutureOr<T> Function(something) and have it decide at runtime whether to return a future or value.
Here, again untraditionally, the framework actually has two paths with different behavior depending on whether it gets a future or not. So you having one function which can return either a value or a future is perfectly suited for that framework.

That doesn't mean that loadLibarary should return a FutureOr and return a non-Future value if called again after the first future has returned. That return type is at an API boundary.

If you want to know whether it has been called, and the returned future has completed, it's just a matter of caching.

Future<void>? Function() libraryLoad = () {
  bool done = false;
  final Future<void> future = prefix.loadLibrary().whenComplete(() {
    done = true
  });
  return () => done ? null : future;
} ();

then you can use libraryLoad() everywhere you'd use a FutureOr-returning prefix.loadLibrary(), and get the desired effect.

Can be made reusable, and configurable (do you want to start loading eagerly when someone creates the function,
which is lazily if stored in a static variable, or only when the function is first called?)

/// Creates a function which caches the result of loading a deferred library.
///
/// When the returned function is called for the first time, the [prefixLoadLibrary] is called,
/// and the future it returns is returned.
/// Further calls of the returned function will return the same future, until that future completes.
/// If the future completes with a value, any further calls to the returned function will return `null`
/// immediately and synchonously. 
/// If the future completes with an error, the returned function will keep returning the same future,
/// with that error, to all later callers.
///
/// The intended use, and where the naming comes from, is to cache the result of the `loadLibrary`
/// function of a deferred library:
/// ```dart
/// import 'package:example/example.dart' deferred as ex;
/// // ...
/// final loadEx = cacheDeferredLibrary(ex.loadLibrary);
/// ```
/// which can then be used as:
/// ```dart
///   if (loadEx() case var future?) await future;
/// ```
/// to wait only if there is a future to wait on, and continue immediately 
/// if the library has already been loaded through that function.
///
/// To trigger loading of the library early, if you know that you'll want to use it later, 
/// you can call the function, and ignore any errors, like `loadEx()?.ignore()`.
FutureOr<void?> Function() cacheDeferredLibrary(Future<void> Function() prefixLoadLibrary) { 
  bool done = false;
  Future<void>? future;
  return () => done ? null : (future ??= (prefixLoadLibrary()..then<void>(
    (_) { done = true; }, 
    onError: (_, _) => future = null)));
}

Your helper function, which runs a callback synchronously or asynchronously, may also be written as an extension method:

import "dart:async";

extension FutureOrThen<T> on FutureOr<T> {
  /// Calls [onValue] on the value of this [FutureOr].
  /// 
  /// Works like [Future.then] on a `Future<T>`, and for the remaining `T` values, 
  /// the result of calling `onValue` with that value is returned directly.
  FutureOr<R> then<R>(
      FutureOr<R> Function(T) onValue, {
      FutureOr<R> Function(Object, StackTrace)? onError,
  }) => switch (this) {
    Future<T> f => f.then<R>(onValue, onError: onError),
    T v => onValue(v),
  };
}

(An extension on FutureOr<T> applies to every type, because FutureOr<X> is a supertype of any type X, so the extension should be imported judiciously. That's also why it will not be part of any platform library.
The only types where it doesn't apply are Future types that have their own then which takes precedence.)

@gmpassos
Copy link
Contributor Author

gmpassos commented Sep 5, 2023

Gratitude for your in-depth response.

I think that the solution could be just some improvements on the standard features (extension) of FutureOr.

To really accomplish what we do (avoid Future and async), we depend on some extensions that assist in writing a clean code:

https://pub.dev/documentation/async_extension/latest/async_extension/FutureOrExtension.html

https://github.com/eneural-net/async_extension/blob/e95fff75dc65c0560f1ca3bebad9890a0eb2db8f/lib/src/async_extension_base.dart#L153

I really think that add at least a standard then extension on dart:async could help a lot. (FYI @kevmoo)

@lrhn
Copy link
Member

lrhn commented Apr 9, 2025

No language change plans, and if there were any, they would be in the repo in issues like dart-lang/language#2033.

@lrhn lrhn closed this as completed Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
Status: Complete
Development

No branches or pull requests

5 participants