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

Support query mutation #1660

Open rrousselGit opened 2 years ago

rrousselGit commented 2 years ago

A common use case is to trigger side-effects and have the UI listen to the status of that side-effect (such as showing a spinner/snackbar on error)

I don't think there is a neat way to support this without code-generation that wouldn't have fundamental flaws (like the inability to pass parameters).
But with the new code-generation syntax for providers, Riverpod should be able to support this

In particular, we could have:

@riverpod
class Example extends _$Example {
  @override
  Future<List<Todo>> build() => fetchTodoList();

  @mutation
  Future<void> addTodo(Todo todo) async {
    await http.post(...., todo.toJson());
  }
}

which would then be used inside widgets by doing:

Consumer(
  builder: (context, ref, child) {
    // as usual, nothing special
    List<Todo> todos = ref.watch(exampleProvider);

    // our UI listens to "addTodo" side-effects.
    // The returned AddTodoMutation object contains things like loading/error/result
    // of the mutation, similar to that of AsyncValue.
    AddTodoMutation addTodo = ref.watch(exampleProvider.addTodo);

    return SomeButton(
      // invoke the Example.addTodo method
      onTap: () => addTodo(Todo())
    );
  },
)

Mutations would also receive a custom ProviderObserver event for start/completion/failure, with an Invocation corresponding to the method invocation.

And of course, riverpod_graph & the devtool should be updated to show mutations

rrousselGit commented 2 years ago

Tagged as "2.1" but likely will come after. Consider this as a "planned"

TimWhiting commented 2 years ago

As a side note, the linter is too aggressive about read vs watch in provider definitions when you are using FutureProvider for mutations. (you don't always want to rerun a mutation when things change, in some cases it might make sense, but not always).

rrousselGit commented 2 years ago

You're not supposed to use FutureProvider for mutations to begin with.

AhmedLSayed9 commented 1 year ago

Will this solve use cases when there's just a single mutation without the need of build?

i.e: a LoginProvider use case which might be a FutureProvider. It will be triggered through onPressed later while you need to pre watch/listen to show a spinner/snackbar on error.

Currently, This can be handled by 3 approaches:

1- Using StateNotifier or now AsyncNotifier with AsyncValue<void> or nullable AsyncValue.

2- Creating 3 providers:

  1. The main FutureProvider that trigger the call.
  2. StateProvider that hold the login parameters which will be initially null.
  3. Basic Provider that return AsyncValue which will be initially AsyncData(null) and it'll watch for the second provider (login params) and will trigger the call when login parameters is submitted.

3- Similar to second approach but instead of creating 3 providers, create just the main provider and handle what second and third providers is doing in the widget itself using Hooks or StatefulWidget.

Update: 3rd approach looks much complicated and will include unnecessary rebuilds of the widget, 1st approach is not favorable and is hard to maintain later. so I think the 2nd approach of combining 3 providers is the way to go (at least for now).

AhmedLSayed9 commented 1 year ago

Edit: check my updated approach.

This an example of the 2nd approach using the 3 providers (which was recommended before on Discord):

final selectedProfileImageProvider = StateProvider.autoDispose<File?>((ref) {
  return null;
});

final updateProfileImageStatusProvider =
    Provider.autoDispose<AsyncValue<void>>((ref) {
  final selectedImage = ref.watch(selectedProfileImageProvider);
  if (selectedImage == null) return const AsyncData(null);

  return ref.watch(updateProfileImageProvider(selectedImage));
});

final updateProfileImageProvider =
    FutureProvider.autoDispose.family<void, File>((ref, imageFile) async {
  final result = await updateProfileImageCall(imageFile);
});

class UserProfileExample extends HookConsumerWidget {
  const UserProfileExample({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context, ref) {
    ref.listen<AsyncValue<void>>(
      updateProfileImageStatusProvider,
      (prevState, newState) {
        prevState?.unwrapPrevious().whenOrNull(
              loading: () => DismissDialog(context),
            );
        newState.unwrapPrevious().whenOrNull(
              loading: () => ShowLoadingDialog(context),
              error: (err, st) => ShowErrorDialog(err),
            );
      },
    );

    pickImage(PickSource pickSource) async {
      final bool canSubmit =
          !ref.read(updateProfileImageStateProvider).isLoading;

      if (canSubmit) {
        try {
          final image =
              await ref.read(pickProfileImageProvider(pickSource).future);
          ref
              .read(updateProfileImageEventProvider.notifier)
              .update((_) => Some(Event.unique(image)));
        } catch (_) {}
      }
    }

    return ImagePickComponent(
      pickFromCameraFunction: () {
        pickImage(PickSource.camera);
      },
      pickFromGalleryFunction: () {
        pickImage(PickSource.gallery);
      },
    );
  }
}

Result:

https://user-images.githubusercontent.com/70890146/204165587-031c8b6a-bee3-4819-be06-f5b940e71be7.mov

This is same as login/register/logout/et. use cases. It works great but looks like too much work to pre watch/listen a callable Future and act upon its state.

ruanwenfeng commented 1 year ago

When can we use this amazing feature, very much looking forward to.

RodolfoSilva commented 1 year ago

Whats is the best alternative until we have the @mutation? @rrousselGit ? Right now I'm using the approach showed by @AhmedLSayed9.

RodolfoSilva commented 1 year ago

We could also use this approach using flutter_hooks, like is made in the React world.

import 'package:flutter/material.dart';

final mutating = useState<bool>(false);
final mutatingError = useState<String?>(null);

final addTodoMutation = useCallback(async () {
  try {
    mutating.value = true;
    mutatingError.value = null;
    // PUT YOU ASYNC LOGIC HERE...
    http.post(....)
    ref.refresh(todosProvider);
  } catch (e) {
    mutatingError.value = e.toString();
  } finally {
    mutating.value = false;
  }
}, [ref]);

if (mutating.value) {
  return Text('Mutating');
} else {
  return ElevatedButton(
    child:  Text('Loading'),
    onPressed: () => addTodoMutation(),
  );
}

But I think this could always return a generic instance of Mutation:

class MutationState<T> {
  MutationState._(this._loading, [this._data, this._error]);

  final bool _loading;
  final T? _data;
  final dynamic _error;

  T? get data => _data;
  bool get isEmpty => _data == null;
  bool get isNotEmpty => _data != null;
  bool get isLoading => _loading;
  bool get isError => !_loading && _error != null;

  factory MutationState.error(dynamic error) {
    return MutationState._(false, null, error);
  }

  factory MutationState.success(T data) {
    return MutationState._(false, data);
  }

  factory MutationState.loading() {
    return MutationState._(true);
  }

  factory MutationState.idle() {
    return MutationState._(false);
  }
}

And refactor the code like that:

import 'package:flutter/material.dart';

final state = useState<MutationState<dynamic>>(MutationState.idle()); 

final addTodoMutation = useCallback(async () {
  try {
    state.value = MutationState.loading();
    // PUT YOU ASYNC LOGIC HERE...
    final result = await http.post(....)
    ref.refresh(todosProvider);
    state.value = MutationState.success(result);
  } catch (e) {
    state.value = MutationState.error(e);
  } 
}, [ref]);

if (state.value.isLoading) {
  return Text('Mutating');
} else {
  return ElevatedButton(
    child:  Text('Loading'),
    onPressed: () => addTodoMutation(),
  );
}

I've not run this code, but I don't think this is working. The point here is to have an Idea of an workaround.

TekExplorer commented 1 year ago

I currently use this provider for mutations (and yes, the FutureOr is necessary, though you could change it to Future and mark it async)

import 'package:riverpod_annotation/riverpod_annotation.dart';

part 'mutation_provider.g.dart';

@riverpod
class Mutation extends _$Mutation {
  @override
  FutureOr<MutationEnum> build(Object mutationKey) => MutationEnum.initial;

  Future<void> call(Future<void> Function() callback) async {
    state = const AsyncValue.loading();
    final result = await AsyncValue.guard<void>(() => callback());
    if (result is AsyncData) {
      state = const AsyncValue.data(MutationEnum.success);
    } else if (result is AsyncError) {
      state = AsyncValue.error(result.error, result.stackTrace);
    }
  }
}

enum MutationEnum {
  initial,
  success;

  bool get isInitial => this == MutationEnum.initial;
  bool get isSuccess => this == MutationEnum.success;
}

i then use it by providing the family with final mutationKey = useMemoized(Object.new)

you can also add whatever convenience methods you like, such as a when on the enum, or creating a mutationWhen extension on AsyncValue<MutationEnum> which replaces data with initial and success

TekExplorer commented 1 year ago

you can always try and adjust it to return the success data if you want, but ideally you should be calling notifier methods from other providers that handle the data for you.

its used by

final mutationKey = useMemoized(Object.new);
final AsyncValue<MutationEnum> mutationState = ref.watch(mutationProvider(mutationKey));
// dialogs or whatever
ref.listen(mutationProvider(mutationKey), (prev, next) {
   // do whatever you like with next (is AsyncError or when or whatever)
});
// handle the state however you like

// later
final mutation = ref.read(mutationProvider(mutationKey).notifier);
//call
mutation(() async {
  await ref.read(someProvider.notifier).someMutation();
});

you could also save it as a callback somewhere

Future<void> doSomething() async => ref
    .read(mutationProvider(mutationKey).notifier)
    .call(() async => ref.read(somethingProvider.notifier).doSomething());
TekExplorer commented 1 year ago

I've improved my approach a lot, with some suggestions by remi. This mirrors the approach that the code gen solution will have, at least as far as api is concerned https://github.com/TekExplorer/riverpod_mutations

it also includes a generic mutations provider.

chimon2000 commented 1 year ago

An interesting solution I discovered last week

https://pub.dev/packages/rivertion

TekExplorer commented 1 year ago

An interesting solution I discovered last week

https://pub.dev/packages/rivertion

missing github link == yeet

chimon2000 commented 1 year ago

https://github.com/BreX900/query/tree/main/rivertion

njwandroid commented 1 year ago

any update? i see this https://docs-v2.riverpod.dev/docs/essentials/side_effects and it seems related but still not quite clear.

TekExplorer commented 1 year ago

any update? i see this https://docs-v2.riverpod.dev/docs/essentials/side_effects and it seems related but still not quite clear.

Not quite what this issue is for. Right now, my riverpod_mutations package is pretty much the closest we have.

TekExplorer commented 1 year ago

For those who care to try early code, consider trying out this generator

dependencies:
  riverpod_mutations_annotation:
    git:
      url: https://github.com/TekExplorer/riverpod_mutations_annotation
dev_dependencies:
  riverpod_mutations_generator:
    git:
      url: https://github.com/TekExplorer/riverpod_mutations_generator

It should be functional. I just need some feedback before i go to publish as a package

neiljaywarner commented 1 year ago

@TekExplorer isn't this the same as https://pub.dev/packages/riverpod_mutations which is already published as a package? is it working well? has anyone else tried it?

@rrousselGit can you confirm this is the api you are planning to take? it looks fairly promising, i am not used to .create lately with riverpod but it seems ok

7 months is a fair bit of time but i i imagine there's a lot going on with riverpod esp with the autodispose/dispose combining

rrousselGit commented 1 year ago

That's not the API I want, no.

The API I want is already in the top post.

neiljaywarner commented 1 year ago

Thanks for clarifying. I'm definitely looking forward to it!

TekExplorer commented 1 year ago

@njwandroid it is not the same no. My generator actually produces the correct API. Give it a try and let me know how it goes!

neiljaywarner commented 1 year ago

Can you please check/update the readme when you get a chance? Unless I'm confused.. it looks different to me..

....sent from my phone

On Mon, Oct 23, 2023, 1:38 PM Omar Kamel @.***> wrote:

@njwandroid https://github.com/njwandroid it is not the same no. My generator actually produces the correct API. Give it a try and let me know how it goes!

— Reply to this email directly, view it on GitHub https://github.com/rrousselGit/riverpod/issues/1660#issuecomment-1775791720, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXBGYM6DLZZA2BPUDVGJF3YA22Q7AVCNFSM6AAAAAAQSJ7QEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZVG44TCNZSGA . You are receiving this because you commented.Message ID: @.***>

TekExplorer commented 1 year ago

@njwandroid

i've updated the generator's readme. You literally use it as demonstrated in the main message of this issue

TekExplorer commented 1 year ago

I've published the generator and annotation as packages on pub.dev!

It not only supports this api, it also supports marking some of your method's parameters as family keys! That allows you to listen on multiple calls (say, you were deleting multiple books by their id and wanted to show a loading indicator for each one, which was not possible before)

https://pub.dev/packages/riverpod_mutations_generator << see the readme https://pub.dev/packages/riverpod_mutations_annotation

ewilliams-zoot commented 10 months ago

I think I saw hints in @TekExplorer 's comments about using mutation keys, which is essential for lists of items that each trigger the same mutation but should only react to UI changes individually. Is that being considered in the solution for this? For example, how would a deleteTodo mutation look in your syntax, @rrousselGit ? Something like this?

  @mutation(useKey: true)
  Future<void> deleteTodo(String todoUid) async {
    await http.delete(...., todoUid);
  }

// in Widget user interaction, only listens to mutations with a key of "this" todo item
AddTodoMutation addTodo = ref.watch(exampleProvider.deleteTodo(todoUid));
TekExplorer commented 10 months ago

Not likely. It's possible to have multiple parameters that want to be keys. My solution uses an additional annotation called @mutationKey on the parameters in question that does this.

ewilliams-zoot commented 10 months ago

That actually makes sense - like a compound key for a database table. An additional annotation would seem to make the most sense in a solution like this then.

mrverdant13 commented 10 months ago

I really find the initial @rrousselGit's proposal quite useful

Being able to discriminate among different mutation methods is currently not possible within a ProviderObserver, which is often essential to register/post the mutation intent (as well as its payload and metadata) to an analytics manager/provider/service.

In the meantime, the approach that best suits my requirements is the use of one Notifier per each mutation method. The Notifier manages a custom variation of an AsyncValue, where the AsyncLoading subclass encloses the metadata related to the ongoing mutation. As a result, any ProviderObserver can be fully aware of the mutation state as well as of its corresponding metadata/payload. Even better, the use of an AnalyticsEvent mixin class to mark those custom classes as such, which later on can be easily identified by the observer.

My guess is that, one of the potential benefits that entails the introduction of the @mutation annotation, is the capability to track them in a similar fashion as described above, but with the additional advantage of the proper automatic update of the root AsyncNotifier state.


I opened a discussion entry looking for existing options for the specific use case I mention here, where I also included a simple example description, in case more context is required.

rrousselGit commented 10 months ago

Yes, tracking what triggered a state change is in the scope of this issue. I specifically am working on that for the devtool, but logging is another one.

RodolfoSilva commented 4 months ago

Hook alternative:

typedef Mutation = FutureOr<dynamic> Function();

(AsyncSnapshot<void>, Mutation) useMutation(
  Mutation callback, [
  List<Object?> keys = const <Object>[],
]) {
  final pendingMutation = useState<Future<void>?>(null);
  final snapshot = useFuture(pendingMutation.value);

  final mutate = useCallback(() {
    pendingMutation.value = Future.microtask(() => callback());
    return pendingMutation.value;
  }, keys);

  return (snapshot, mutate);
}

typedef MutationFamily<T> = FutureOr<dynamic> Function(T params);

(AsyncSnapshot<void>, MutationFamily<T>) useMutationFamily<T>(
  MutationFamily<T> callback, [
  List<Object?> keys = const <Object>[],
]) {
  final pendingMutation = useState<Future<void>?>(null);
  final snapshot = useFuture(pendingMutation.value);

  final mutate = useCallback((params) {
    pendingMutation.value = Future.microtask(() => callback(params));
    return pendingMutation.value;
  }, keys);

  return (snapshot, mutate);
}

@rrousselGit do you have any suggestion?

masreplay commented 4 months ago

I created this simple package using Riverpod and Hook riverpod_hook_mutation

Define

final addTodo = useMutation<TODO>();

Call

addTodo(ref.read(provider.notifier).addTodo())

Usage

addTodo.when(
  idle: () => const Icon(Icons.add),
  data: (data) => const Icon(Icons.add),
  error: (error, stackTrace) => const Icon(Icons.add_circle_outline),
  loading: () => const CircularProgressIndicator(),
)
TekExplorer commented 4 months ago

I wouldn't have named it that. It's not riverpod specific.