rrousselGit / riverpod

A reactive caching and data-binding framework. Riverpod makes working with asynchronous code a breeze.
https://riverpod.dev
MIT License
6.28k stars 955 forks source link

Error: The argument type 'ScopedProvider<FirestoreDatabase>' can't be assigned to the parameter type 'AlwaysAliveProviderBase<Object, FirestoreDatabase>' #144

Closed bizz84 closed 4 years ago

bizz84 commented 4 years ago

Describe the bug Currently unable to get data from a StreamProvider that is created from a stream that comes from a ScopedProvider.

Note: I may be using this wrong. If so, apologies. In any case I hope this can help to improve the documentation.

To Reproduce

Here's some minimal code from my app:

class FirestoreDatabase {
  FirestoreDatabase({@required this.uid});
  final String uid;

  Stream<List<Job>> jobsStream() => /* reads and parses some stream from Firestore */
}

// created somewhere
final databaseProvider =
    ScopedProvider<FirestoreDatabase>((ref) => throw UnimplementedError());

// then, somewhere else:
ProviderScope(
        overrides: [
          databaseProvider
              .overrideAs((watch) => FirestoreDatabase(uid: user.uid)),
        ],
 child: JobsPage());

// inside JobsPage:
class JobsPage extends ConsumerWidget {
  Widget build(BuildContext context, ScopedReader watch) {
    final jobsStreamProvider = StreamProvider<List<Job>>(
      (ref) => ref.watch<FirestoreDatabase>(databaseProvider).jobsStream(),
    );
    final jobsStream = watch(jobsStreamProvider);
    return jobsStream.when(
      data: (data) => SomeWidget(data),
      loading: () => CircularProgressIndicator(),
      error: (_, __) => Text('error')
    );
   }
}

I'm currently experiencing two problems with the code above.

If I create the jobsStreamProvider like this (by using the watch argument from the build method):

    final jobsStreamProvider = StreamProvider<List<Job>>(
      (ref) => watch<FirestoreDatabase>(databaseProvider).jobsStream(),
    );

then the code compiles and runs, but the UI is stuck in the loading state (that is, I never get data inside jobsStream.when).

If instead I try to use ref to get the databaseProvider:

    final jobsStreamProvider = StreamProvider<List<Job>>(
      (ref) => ref.watch<FirestoreDatabase>(databaseProvider).jobsStream(),
    );

then I get the following compile error:

lib/app/home/jobs/jobs_page.dart:49:45: Error: The argument type 'ScopedProvider<FirestoreDatabase>' can't be assigned to the parameter type 'AlwaysAliveProviderBase<Object, FirestoreDatabase>'.
 - 'ScopedProvider' is from 'package:riverpod/src/framework.dart' ('../../../../flutter/flutter_beta/.pub-cache/hosted/pub.dartlang.org/riverpod-0.10.0/lib/src/framework.dart').
 - 'FirestoreDatabase' is from 'package:starter_architecture_flutter_firebase/services/firestore_database.dart' ('lib/services/firestore_database.dart').
 - 'AlwaysAliveProviderBase' is from 'package:riverpod/src/framework.dart' ('../../../../flutter/flutter_beta/.pub-cache/hosted/pub.dartlang.org/riverpod-0.10.0/lib/src/framework.dart').
 - 'Object' is from 'dart:core'.
      (ref) => ref.watch<FirestoreDatabase>(databaseProvider).jobsStream(),
                                            ^

FAILURE: Build failed with an exception.

Additional questions:

Expected behavior No compile error, StreamProvider produces data rather than being stuck on loading state.

TimWhiting commented 4 years ago

Maybe I can answer some of this.

ScopedProviders can only be read within the UI tree. They cannot be obtained from the ref parameter in provider creation. That is why it is a ScopedReader that you get in the UI tree, and ref.read is typedef'd as Reader. TLDR: ScopedReaders can read ScopedProviders + regular Providers, Readers can only read regular Providers

The best way to deal with scoped providers providing parameters to other providers is to use a Family provider.

// in the top level of the file
 final jobsStreamProvider = StreamProvider.family<List<Job>, FirestoreDatabase>(
      (ref, database) => database.jobsStream(),
    );
// in your Jobs Page widget
final asyncValue = watch(jobsStreamProvider(watch(databaseProvider)));

It's not a problem to have providers in the top level of the file. They don't use memory until they are used, and with the dispose modifier they are cleaned up. If you want to make it private to the file to avoid polluting the global scope that is also a good option.

However, My personal preference is to use a StateProvider for configuration, and then updating that state provider rather than using overrides. I personally avoid overrides and ScopedProvider partly for this reason. The other advantage is that the definition of the jobs provider itself depends on the configuration state and can watch and update if you change the configuration (even in a different part of the app).

final databaseProvider =
    StateProvider<FirestoreDatabase>((ref) =>null);

// Instead of the override:
// Use a hook or something in initState of the widget 
// where you are doing the override
// and update the state of the databaseProvider

// i.e. (hooks) in the build() method
useMemoized((_) {
  context.read(databaseProvider).state = FirestoreDatabase(uid: user.uid);
});
// (no hooks) (I could be wrong on this since I use hooks
initState() {
   context.read(databaseProvider).state = FirestoreDatabase(uid: user.uid);
}
// Note this update logic of the databaseProvider state could also be anywhere else such as in 
// an onTap or other such reaction to user input

final jobsStreamProvider = StreamProvider.family<List<Job>>(
     (ref) => ref.watch(databaseProvider).jobsStream(),   
);

// in your Jobs Page widget
final asyncValue = watch(jobsStreamProvider));
rrousselGit commented 4 years ago

@TimWhiting is correct, especially on:

ScopedProviders can only be read within the UI tree.

(Although ScopedProviders can read other ScopedProviders if they need to)

In general, ScopedProvider is quite a niche. It's here mostly for rebuild optimizations and custom themes.

He is also correct in suggesting StateProvider, which is usually the easiest solution.

rrousselGit commented 4 years ago

When should one use ref.watch as opposed to just watch from a Consumer or ConsumerWidget?

The answer should come naturally.

Providers have only access to ref.watch. Widgets don't have access to ref.watch. There's never really a time where you have to pick between one or another. There's only one choice, based on where you are.

TimWhiting commented 4 years ago

@rrousselGit Maybe a good lint would be for this case where a user is trying to use the watch method of the Consumer or ConsumerWidget inside a definition of a provider...

TimWhiting commented 4 years ago

Though I guess you do get the compile time error already.

rrousselGit commented 4 years ago

Maybe a good lint would be for this case where a user is trying to use the watch method of the Consumer or ConsumerWidget inside a definition of a provider...

How is this possible? From my understanding, people would have to go really out of their way to use Consumer/ConsumerWidget's watch method inside providers.

It is likely that @bizz84's confusion is caused by how he declares his providers directly in the build method. But that shouldn't be allowed.

I'll definitely add a lint against writing:

Widget buid(...) {
  final provider = Provider(..);
}
TimWhiting commented 4 years ago

@rrousselGit That is what I meant, providers should not be in the build method. That is a better more general lint.

I think it might be helpful to have some documentation on how to think about architecture in an app using Riverpod. The way I think about it is to analyze the dataflow.

So in this case we have: jobStream: depends on firebaseDatabase firebaseDatabase: depends on User/uid .... etc

If you can create this dependency graph for the dataflow: Then it is as easy as creating a provider for each node, and watching all dependencies. It gets a little tricker when you deal with asynchronous dependencies / asynchronous configuration, but there are ways of making that synchronous through using a StateProvider and having the asynchronous operation update the StateProvider. If you do this, you can make your whole dependency graph synchronous which avoids a lot of headaches that others have run into and posted issues on.

I think this is understood by you and me, but I don't think it is explicitly mentioned or pointed out anywhere. The focus of the documentation is currently more on

  1. The advantages over Provider (compile safe, etc).
  2. How to use it.
  3. The different types of providers and variants.

But I haven't seen as much on

  1. good practices
  2. how to think about architecture with riverpod
  3. design patterns

I think this is why there are a lot of questions in the issues that are not actual bugs, but misunderstandings on how to use it in a good way.

rrousselGit commented 4 years ago

Agreed. My main issue is, for me, using Riverpod is natural. So I have a real difficulty writing documentation before people raise issues.

I'll definitely improve the documentation when I can. But for now, I need to gather more data on how people naturally think with Riverpod.

TimWhiting commented 4 years ago

I understand.

I'm an PhD Student, and currently we are onboarding a few new people in our research lab. I'll definitely take notes on what can be improved in the documentation from the things they run into. I can also help with documentation a bit. Maybe next week I can submit a PR with some documentation or a Cookbook looking at it from the dataflow perspective, and maybe sometime I'll have some time to write some documentation on making asynchronous dependency chains synchronous via StateProvider.

rrousselGit commented 4 years ago

That would be lovely, thanks in advance!

smiLLe commented 4 years ago

but there are ways of making that synchronous through using a StateProvider and having the asynchronous operation update the StateProvider

Is this the recommended way you describe?

The example below will become a bug in my code if i am going to use becomeSync before i used fut provider. So i either have to assert if null or my ctrl must have a default value. Why not make becomeSync a Family?

final ctrl = StateController<Foo>();
final fut = FutureProvider((ref) async {
  final data = await doStuff();
  ref.read(ctrl).state = data;
  return data;
});
final becomeSync = Provider<Foo>((ref) {
  return ref.watch(ctrl.state);
});
rrousselGit commented 4 years ago
final ctrl = StateProvider<Foo>((_) => Foo());

final fut = FutureProvider((ref) async {
  final data = await doStuff();
  ref.read(ctrl).state = data;
  return data;
});

Don't do that This will definitely have a lint telling you not do to that, and if I could, I would throw an exception.

Reverse the dependency instead:

final ctrl = StateProvider<Foo>((ref) {
  final foo = Foo();
  ref.watch(ctrl.future).then((value) => foo.state = value);
  return foo;
});

That makes the state declarative and with a uni-directional data-flow.

That could be critical if, say, ctrl was using autoDispose.

bizz84 commented 4 years ago

@TimWhiting @rrousselGit thanks a lot for all the answers. These clear some confusion for me.

I have some follow-up questions about asynchronous dependencies at runtime.

In my app I was using the data from a StreamProvider to update the databaseProvider (with overrideAs):

final authStateChangesProvider = StreamProvider<User>(
    (ref) => ref.watch(firebaseAuthProvider).authStateChanges());

class AuthWidget extends ConsumerWidget {

  @override
  Widget build(BuildContext context, ScopedReader watch) {
    final authStateChanges = watch(authStateChangesProvider);
    return authStateChanges.when<Widget>(
      data: (user) => _data(context, user),
      loading: () => const Scaffold(
        body: Center(
          child: CircularProgressIndicator(),
        ),
      ),
      error: (_, __) => const Scaffold(
        body: Center(
          child: Text('Error reading authStateChanges()'),
        ),
      ),
    );
  }

  Widget _data(BuildContext context, User user) {
    if (user != null) {
      return ProviderScope(
        overrides: [
          databaseProvider
              .overrideAs((watch) => FirestoreDatabase(uid: user.uid)),
        ],
        child: HomePage(),
      );
    }
    return SignInPage();
  }
}

The suggested alternatives were to:

  1. use hooks to update the databaseProvider's state inside the build method:
useMemoized((_) {
  context.read(databaseProvider).state = FirestoreDatabase(uid: user.uid);
});
  1. Do the same in initState() without hooks:
initState() {
   context.read(databaseProvider).state = FirestoreDatabase(uid: user.uid);
}

I dislike 2) because I'd like to keep my setup with a StreamProvider inside a StatelessWidget, rather than having to create one new stateful widget just so that I can update the state in initState().

I'm not sure how 1) would work in practice. Should my AuthWidget extend HookWidget so that I can use useMemoized? Should I add hooks_riverpod as a dependency in order to enable this? The documentation for hooks_riverpod only shows useProvider & I can't find any references to useMemoized.

Maybe my goal could be rephrased as: How can I update a StateProvider's state when a StreamProvider emits a new value, with minimal boilerplate?

rrousselGit commented 4 years ago

Ah I answered that in your PR before seeing your comment.

TL;DR:

final databaseProvider = Provider<FirestoreDatabase>((ref) {
  final auth = ref.watch(authStateChangesProvider);

  if (auth.data?.value?.user?.id != null) {
    return FirestoreDatabase(auth.data.value.user.id);
  }
  return null;  
});

Benefits:

bizz84 commented 4 years ago

@rrousselGit Ah yes, I think I missed that in the previous comments.

Looks very clean, everything in one place. I like it!

One question: what happens when there are some listeners to some of the streams in the database, and the user logs out?

Would any dependant StreamProviders emit an error once the database becomes null again?

Just wondering how things look from a lifecycle point of view when the conditions inside the providers change.

rrousselGit commented 4 years ago

what happens when there are some listeners to some of the streams in the database, and the user logs out?

When the user logs out, databaseProvider will rebuild (the function is called again), which will return null. Then, everything that listens to databaseProvider will update too.

It's kinda magical. Everything that needs to rebuild do so automatically :shrug:

bizz84 commented 4 years ago

When the user logs out, databaseProvider will rebuild (the function is called again), which will return null. Then, everything that listens to databaseProvider will update too.

Suppose I have this code:

final jobsStreamProvider = StreamProvider.family<List<Job>, FirestoreDatabase>(
  (ref, database) => database.jobsStream(),
);

// in build method:
final jobsStream = watch(jobsStreamProvider(watch(databaseProvider)));

Should I change database.jobsStream() to database?.jobsStream() to account for the fact that the database can be null?

If so, should I null check this variable before using it?

    final jobsStream = watch(jobsStreamProvider(watch(databaseProvider)));

It does indeed seem a big magical - though I want to make sure I'm using it correctly.

rrousselGit commented 4 years ago

Suppose I have this code:

final jobsStreamProvider = StreamProvider.family<List<Job>, FirestoreDatabase>(
  (ref, database) => database.jobsStream(),
);

// in build method:
final jobsStream = watch(jobsStreamProvider(watch(databaseProvider)));

To begin with, that initial code looks weird. Why not do:

final jobsStreamProvider = StreamProvider<List<Job>>((ref) {
  final database = ref.watch(databaseProvider);
  return database?.jobsStream() ?? Stream.empty();
});
bizz84 commented 4 years ago

Cool. I think I understand enough now.

All runtime logic that handles the dependencies between providers can go inside the declaration of the providers themselves.

Which is super nice because it's completely separate from the widgets themselves. Good stuff 👍

rrousselGit commented 4 years ago

Depending on what your DB object is doing, you may also want to dispose it to close streams and cancel pending requests. You could do that with:

final databaseProvider = Provider<FirestoreDatabase>((ref) {
  final auth = ref.watch(authStateChangesProvider);

  if (auth.data?.value?.user?.id != null) {
    final db = FirestoreDatabase(auth.data.value.user.id);
    ref.onDispose(db.dispose);
    return db;
  }
  return null;  
});

With this, when the user log-out, this will dispose of the previous DB object

davidmartos96 commented 4 years ago
final databaseProvider = Provider<FirestoreDatabase>((ref) {
  final auth = ref.watch(authStateChangesProvider);

  if (auth.data?.value?.user?.id != null) {
    return FirestoreDatabase(auth.data.value.user.id);
  }
  return null;  
});

@rrousselGit An example like that one or similar would be very welcome in the docs. I happened to come across to a similar need, where I initialize some asynchronous providers before the app starts, but I want to use those providers without handling the future states (loading/error), since they have already been handled when starting the app. On my case it's opening an Sqlite database.

bizz84 commented 4 years ago

@rrousselGit

Depending on what your DB object is doing, you may also want to dispose it to close streams and cancel pending requests.

In my case my DB is stateless and only exposes streams. The StreamProviders are the ones setting up listeners (when used inside widgets). Since they are global, do they automatically know when to dispose the listeners (e.g. when the widgets are unmounted)?

rrousselGit commented 4 years ago

Since they are global, do they automatically know when to dispose the listeners (e.g. when the widgets are unmounted)?

With autoDispose, yes. https://riverpod.dev/docs/concepts/modifiers/auto_dispose But as mind-stretching as it is, the state of providers isn't global.

This matters because of ref.watch.

Say you have:

final a = StreamProvider(...);

final b = WhateverProvider((ref) {
  ref.watch(a);
  ref.onDispose(<do something>);
});

in this situation, when a updates, b will be disposed and a new instance will be created.

That is because we actually have a tree. a is the root node, and b is a child of a.

rrousselGit commented 4 years ago

To reinforce the fact that providers aren't global, we can't do:

final a = WhateverProvider((ref) {
  ref.watch(b);
});

final b = WhateverProvider((ref) {
  ref.watch(a);
});

That will throw a runtime exception because the dependency graph of providers must be uni-directional.

As a consequence, if b depends on a then a cannot depend on b. This shows that we cannot read providers from anywhere (since some providers can't read other providers). So providers actually aren't global state.

rrousselGit commented 4 years ago

That's a quite advanced discussion though. I'm not sure how to explain this concept to a beginner. If you have any idea how to explain that, or if you didn't understand what I meant, I'd love it if you could say so.

Understanding that concept is an important step to mastering the power of ref.watch.

bizz84 commented 4 years ago

@rrousselGit people will be familiar with the widget tree.

So if providers are arranged as a tree-like structure based on their inter-dependencies, you could say so in the documentation.

Maybe it would be useful to see a flow diagram for a simple example app, showing how providers are added/removed from the tree when certain events happen, alongside changes to the widget tree. Though it's just an idea and I'm not sure how it should look in practice.

SalahAdDin commented 3 years ago

Ok, it was very hard to me to understand this since i'm so new in Flutter.

I have the next use case(similar one):

final userRepositoryProvider =
    Provider<UserRepository>((ref) => AmplifyGraphQLUserRepository());

final userEntityProvider = Provider<User>((ref) => User.empty);

final homeScreenProvider = StateNotifierProvider<UserNotifier>((ref) {
  final userEntity = ref.watch(userEntityProvider);
  final userRepository = ref.watch(userRepositoryProvider);
  return UserNotifier(currentUser: userEntity, userRepository: userRepository);
});

I need my userEntity to be provider after logging in, and i tried by overridesWithValue but it gave me a ProviderScope problem, so I opted by follows the @TimWhiting comment and overriding it after loggin the user by:

loggedIn: (loggedInUser) {
            context.read(homeScreenProvider).setCurrentUser(loggedInUser);
            context.read(homeScreenProvider).deliverUserScreen();
            pushAndReplaceToPage(context, HomeScreen());
          },

Is it the best way to work with?

TimWhiting commented 3 years ago

I would make your userEntityProvider a StateProvider and set it's state to the logged in user. It makes more sense there since you won't always be on the home screen, and the homeScreenProvider watches for changes in the userEntityProvider anyways