Skip to content
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

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

Open
saibotma opened this issue Feb 20, 2024 · 13 comments
Assignees
Labels
bug Something isn't working

Comments

@saibotma
Copy link

saibotma commented Feb 20, 2024

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

  • Run the example
  • Spam the keyboard while focusing the text field
  • Notice the exceptions

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 saibotma added bug Something isn't working needs triage labels Feb 20, 2024
@saibotma
Copy link
Author

saibotma commented Feb 20, 2024

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
Copy link
Owner

rrousselGit commented Mar 9, 2024

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
Copy link
Owner

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.

@rrousselGit rrousselGit added this to the Post 3.0 release milestone Mar 10, 2024
@zhxst
Copy link

zhxst commented Mar 11, 2024

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
Copy link
Owner

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
Copy link

Here as well, hopefully it can be resolved

@saibotma
Copy link
Author

saibotma commented Apr 5, 2024

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

@nateshmbhat
Copy link

please thumbs up this issue guys

@dumikaiya
Copy link

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
Copy link

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
Copy link
Owner

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

@dancojocaru2000
Copy link

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants