rrousselGit / riverpod

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

Cannot use ref functions after the dependency of a provider changed but before the provider rebuilt, when chaining providers #3354

Open saibotma opened 9 months ago

saibotma commented 9 months ago

Describe the bug This example has got two providers, one state provider, which persists the entered text, and a view model provider, which forwards the entered text from the state provider & also forwards the input event to the state provider. The example is very stripped down, and obviously you could do the same thing with only one provider, without any issue, however the issue appeared in a more complex real world situation where two providers were necessary.

To Reproduce

The exception disappears when you move the read the notifier inside the onChanged callback or stop watching the view model provider, as indicated in the comments.

import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:riverpod_annotation/riverpod_annotation.dart';

part 'main.g.dart';

@riverpod
class MyState extends _$MyState {
  @override
  String build() => "";

  void setText(String text) {
    state = text;
  }
}

@riverpod
class MyViewModel extends _$MyViewModel {
  @override
  String build() => ref.watch(myStateProvider);

  void enterText(String text) {
    ref.read(myStateProvider.notifier).setText(text);
  }
}

void main() {
  runApp(const ProviderScope(child: MyApp()));
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: MyScreen(),
    );
  }
}

class MyScreen extends ConsumerStatefulWidget {
  const MyScreen({super.key});

  @override
  ConsumerState<MyScreen> createState() => _MyScreenState();
}

class _MyScreenState extends ConsumerState<MyScreen> {
  @override
  Widget build(BuildContext context) {
    // Using this notifier causes the exception.
    final notifier = ref.read(myViewModelProvider.notifier);
    // Commenting this line out makes the exception go away.
    ref.watch(myViewModelProvider);
    return Scaffold(
      body: Center(
        child: TextField(
          onChanged: (it) {
            // Using this notifier does not cause the exception.
            //final notifier = ref.read(myViewModelProvider.notifier);
            notifier.enterText(it);
          },
        ),
      ),
    );
  }
}
======== Exception caught by widgets ===============================================================
The following assertion was thrown while calling onChanged:
Cannot use ref functions after the dependency of a provider changed but before the provider rebuilt
'package:riverpod/src/framework/element.dart':
Failed assertion: line 674 pos 7: '!_didChangeDependency'

When the exception was thrown, this was the stack: 
#2      ProviderElementBase._assertNotOutdated (package:riverpod/src/framework/element.dart:674:7)
#3      ProviderElementBase.read (package:riverpod/src/framework/element.dart:688:5)
#4      MyViewModel.enterText (package:riverpod_bug_1902/main.dart:23:9)
#5      _MyScreenState.build.<anonymous closure> (package:riverpod_bug_1902/main.dart:62:22)
#6      EditableTextState._formatAndSetValue (package:flutter/src/widgets/editable_text.dart:3834:27)
#7      EditableTextState.updateEditingValue (package:flutter/src/widgets/editable_text.dart:3036:7)
#8      TextInput._updateEditingValue (package:flutter/src/services/text_input.dart:2025:43)
#9      TextInput._handleTextInputInvocation (package:flutter/src/services/text_input.dart:1858:29)
#10     TextInput._loudlyHandleTextInputInvocation (package:flutter/src/services/text_input.dart:1759:20)
#11     MethodChannel._handleAsMethodCall (package:flutter/src/services/platform_channel.dart:559:55)
#12     MethodChannel.setMethodCallHandler.<anonymous closure> (package:flutter/src/services/platform_channel.dart:552:34)
#13     _DefaultBinaryMessenger.setMessageHandler.<anonymous closure> (package:flutter/src/services/binding.dart:567:35)
#14     _invoke2 (dart:ui/hooks.dart:344:13)
#15     _ChannelCallbackRecord.invoke (dart:ui/channel_buffers.dart:45:5)
#16     _Channel.push (dart:ui/channel_buffers.dart:135:31)
#17     ChannelBuffers.push (dart:ui/channel_buffers.dart:343:17)
#18     PlatformDispatcher._dispatchPlatformMessage (dart:ui/platform_dispatcher.dart:722:22)
#19     _dispatchPlatformMessage (dart:ui/hooks.dart:257:31)
(elided 2 frames from class _AssertionError)
====================================================================================================

Expected behavior No exception should be thrown.

saibotma commented 9 months ago

After thinking about the issue, I now understand the reason behind the error messages. However, what I still don't understand is what the recommended way to call the notifier is in this case? I did not expect that I may not use the notifier variable from build inside the onChanged callback. I mean it makes sense after thinking about it, but it is not apparent for a beginner, and also not very intuitive. I am happy to enhance the documentation on this topic, after I got confirmation, that my reasoning is correct.

rrousselGit commented 8 months ago

You're correct, this is indeed quite unintuitive. I need to think about the desirable behaviour here.

A simpler reproduction is:

@riverpod
class MyViewModel extends _$MyViewModel {
  @override
  String build() => ref.watch(myStateProvider);

  void crash() {
    ref.read(myStateProvider.notifier).state = 'Foo';
    ref.read(myStateProvider);
  }
}

Although this is consistent with how the assert behaves, that's wayy too inconvenient. I'll change this.

rrousselGit commented 8 months ago

This is probably going to stay like this for some time. I can't immediately think of a good alternative that's not just "disable the assert".

For now, it seems to be a bit niche. If this gets a bunch of 👍 I'll consider it more.

zhxst commented 8 months ago

You're correct, this is indeed quite unintuitive. I need to think about the desirable behaviour here.

A simpler reproduction is:

@riverpod
class MyViewModel extends _$MyViewModel {
  @override
  String build() => ref.watch(myStateProvider);

  void crash() {
    ref.read(myStateProvider.notifier).state = 'Foo';
    ref.read(myStateProvider);
  }
}

Although this is consistent with how the assert behaves, that's wayy too inconvenient. I'll change this.

So we can not read state immediately after change the state. Sometimes I got this exception but didn't know what happened. I thought it should be safe because riverpod was in the same isolate.

rrousselGit commented 8 months ago

Indeed. As I said I'll likely change this, but at the moment I need to think about how. And it doesn't seem like a very active issue, so we'll see

dumikaiya commented 7 months ago

Here as well, hopefully it can be resolved

saibotma commented 7 months ago

@dumikaiya Please thumb up the issue when you support it!

nateshmbhat commented 7 months ago

please thumbs up this issue guys

dumikaiya commented 7 months ago

Also does anyone know a workaround this? Currently I just wrap the ref.read(provider.notifier).state = value in a Future. I think it rebuilds the widget later with the newly added or changed value

MiniSuperDev commented 6 months ago

I currently use something like this, is it okay? or can it cause unexpected errors?

Thanks

@riverpod
class MyViewModel extends _$MyViewModel {
  @override
  String build() => ref.watch(myStateProvider);

  void crash() {
    try{
      ref.read(myStateProvider.notifier).state = 'Foo';
      ref.read(myStateProvider); // usually ref.read(otherNotifier).someMethod();
     } catch (e) {
      if (e is AssertionError) {
        if (e.toString().contains(
            'Cannot use ref functions after the dependency of a provider changed but before the provider rebuilt')) {
        } else {
          rethrow;
        }
      } else {
        rethrow;
      }
    }
  }
}
rrousselGit commented 6 months ago

I'll tackle this in a few weeks. I do plan in fixing this before the official 3.0 release.

dancojocaru2000 commented 5 months ago

Is there a way to disable the assertion until that's done? I can't think of a way to refactor the application to avoid reading from a provider after a set but before a rebuild.

rrousselGit commented 5 months ago

No. To begin with, this isn't a problem in the stable version.

Yegair commented 5 months ago

No. To begin with, this isn't a problem in the stable version.

I am encountering a similar behavior with flutter_riverpod: 2.5.1 and riverpod_generator: 2.4.0, which I assume is what you are referring to as the stable version? I know there is riverpod_generator: 2.4.2, but I can't update right now, so I did not test whether the issue is solved there.

My code roughly looks like this

try {
  await Future.wait([
    ref.read(myNotifierProvider.notifier).doSomethingThatTakesLong(),
    ref.read(myOtherNotifierProvider.notifier).doSomethingThatTakesEvenLonger(),
  ]);
} on Exception {
  // ... do some error handling
}

// crash
final updatedStateOfMyOtherNotifier = ref.read(myOtherNotifierProvider);

Apparently I can work around it like this, but I have no idea if this has other unwanted side effects (besides of being a little slower due to the Future.delayed overhead):

try {
  await Future.wait([
    ref.read(myNotifierProvider.notifier).doSomethingThatTakesLong(),
    ref.read(myOtherNotifierProvider.notifier).doSomethingThatTakesEvenLonger(),
  ]);
} on Exception {
  // ... do some error handling
}

+ await Future.delayed(Duration.zero);

- // crash
final updatedStateOfMyOtherNotifier = ref.read(myOtherNotifierProvider);
ArbazIrshad commented 4 months ago

I have also encountered this error when using riverpod with go_router. I am also watching my app initialization in the go router provider. But on initial startup, I get this error.

My go_router_provider:

@riverpod
GoRouter router(RouterRef ref) {
  final appStartup = ref.watch(appStartupProvider);
  final authRepository = ref.watch(authRepositoryProvider);
  return GoRouter(
    debugLogDiagnostics: kDebugMode,
    initialLocation: authRepository.isLoggedIn
        ? AppRoutes.home.path
        : AppRoutes.initial.path,
    refreshListenable: GoRouterRefreshStream(authRepository.authState()),
    redirect: (context, state) {
      final user = ref.read(authRepositoryProvider).getSignedInUser();
      return null;
    },
    routes: [
      GoRoute(
        name: AppRoutes.initial.name,
        path: AppRoutes.initial.path,
        builder: SplashView.route,
        routes: [
          GoRoute(
            name: AppRoutes.email.name,
            path: AppRoutes.email.path,
            builder: EnterEmailView.route,
            routes: [
              GoRoute(
                name: AppRoutes.otp.name,
                path: AppRoutes.otp.path,
                builder: OtpView.route,
              ),
            ],
          ),
        ],
      ),
      GoRoute(
        name: AppRoutes.home.name,
        path: AppRoutes.home.path,
        builder: HomeView.route,
      ),
    ],
  );
}

My Startup Code(nothing is there yet.):

@Riverpod(keepAlive: true)
 Future<void> appStartup(AppStartupRef ref) async {}

For now, I am going to remove the appstartup from here. and make a separate loading widget. Is there any other way to handle this?

rrousselGit commented 4 months ago

Fwiw I'm currently working on fixing riverpod_lint, because analyzer broke custom_lint It's going to take a bit of time.

But just expect this error to go away.

pisaq commented 3 months ago

Hi @rrousselGit any progress :) ?

rrousselGit commented 3 months ago

You're using a dev branch. Expect bugs for some time ;)

I'm currently fixing custom_lint because it stopped working.

lennart-devhelden commented 2 months ago

What is the current situation regarding this issue? Using the newest version of the package (v2.5.1) this behavior still occurs when calling a notifier function and then using a ref. It makes some code in our codebase really complicated. Thanks for any updates :)

rrousselGit commented 2 months ago

This issue is specifically about the dev branch. If you have this using the stable version, that thread doesn't apply.

Consider creating a new issue and explain how you got this error.

lennart-devhelden commented 2 months ago

What do you mean with dev branch? I'm using the newest pub.dev version from flutter_riverpod which is 2.5.1 and this error still occurs. I don't think that the official version on pub.dev is your dev branch, right? When it is can you please guide me how I can get the stable version of the package.

rrousselGit commented 2 months ago

You're not on the dev branch. But this issue is about the dev branch.