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

Add a migration page for "Migrating from StateNotifier to Async/Notifier" #2630

Closed
9 tasks
rrousselGit opened this issue Jun 9, 2023 · 6 comments · Fixed by #2980
Closed
9 tasks

Add a migration page for "Migrating from StateNotifier to Async/Notifier" #2630

rrousselGit opened this issue Jun 9, 2023 · 6 comments · Fixed by #2980
Labels
documentation-rework Internal label for documentation-related work

Comments

@rrousselGit
Copy link
Owner

rrousselGit commented Jun 9, 2023

This is a meta issue for tracking documentation work related to migrating from StateNotifier to Async/Notifier

Cf #2504 (comment)
Cf #1767


  • Cross-link this document in the Provider migration guide, in the section talking about "Use ChangeNotifierProvider at first"
    cf https://github.com/rrousselGit/riverpod/pull/2676/files#r1260743192

  • StateNotifier.addListener and .stream aren't present inside Notifier.
    If you want to listen to a Notifier, you can use providerContainer.listen(provider) or ref.listen(provider).

  • StateNotifier.debugState no longer exists.
    Tests which want to interact with Notifiers should always do so by reading the provider instead of directly instantiating the Notifier by hand.
    You can do providerContainer.read(provider) to get the current state of a provider inside tests.

  • Document that Async/NotifierProviders do not receive a "ref" parameter.
    Logic that you would typically place inside the provider should be moved to the Notifier.build method, which has access to both ref and potential family parameters.
    Cf NotifierProvider misses ref #2460

  • Document that the Async/Notifier type is bound to autoDispose/family features. Enabling/disabling those notifiers means you should change the subclassed notifier type:

    Provider Notifier
    NotifierProvider<MyNotifier, MyState>(MyNotifier.new) class MyNotifier extends Notifier {
    NotifierProvider.autoDispose<MyNotifier, MyState>( MyNotifier.new) class MyNotifier extends Notifier {
    NotifierProvider.autoDispose.family<MyNotifier, MyState, Arg>( MyNotifier.new ) class MyNotifier extends AutoDisposeFamilyNotifier<MyState, Arg> {
    NotifierProvider.family<MyNotifier, MyState, Arg>( MyNotifier.new ) class MyNotifier extends FamilyNotifier<MyState, Arg> {

    Note: AutoDispose variants will likely disappear once Should the type difference between autoDispose and non-autoDispose providers be removed? #2576 lands

  • Document how Notifiers are not disposed on ref.watch/invalidate/...
    As such, ref.watch(provider.notifier) never emits a new value.
    This enables keeping state across provider rebuilds. For example, we can do:

    class MyNotifier extends Notifier<int> {
       bool _isFirstBuild = true;
      @override
      int build() {
        if (_isFirstBuild) {
          // TODO maybe read state from the local database
          _isFirstBuild = false;
        }
      }
    }

    This also means that instead of overriding dispose, notifiers should instead rely on ref.onDispose.

  • Due to the difference in dispose life-cycle, there is no-longer a "mounted" property as a boolean wouldn't quite work.
    Related features: ref.onDispose & other life-cycles listeners should offer a way to remove the listener #2668 & Support query mutation #1660

  • How to override "dispose"? Instead use ref.onDispose

  • How to run logic after "build"

    class QrScanner extends StateNotifier<QrScannerState> {
      QrScanner() : super(const QrScannerState()) {
        _startScanner();
      }
    }

    vs:

    @riverpod
    class QrScanner extends _$QrScanner {
      @override
      QrScannerState build() {
       state = const QrScannerState();
       _startScanner();
       return state;
      }
    }
@rrousselGit rrousselGit added the documentation Improvements or additions to documentation label Jun 9, 2023
@nilsreichardt
Copy link
Contributor

It would be great to mention in this page (or a different), that you can pass parameters to the build method (see #2628 (comment))

@rrousselGit rrousselGit added documentation-rework Internal label for documentation-related work and removed documentation Improvements or additions to documentation labels Jun 20, 2023
@lucavenir
Copy link
Contributor

I think it's best to write a migration page "From Riverpod 1.0 to Riverpod 2.0".

That's when Notifier was introduced. And also other stuff happened as well. Is this being tracked? Or can we change this issue to be wider and track down this whole topic, instead?

My intention is to fill in another PR about this topic, so that I can cross reference it into #2676 and submit it.

@rrousselGit
Copy link
Owner Author

I'd be skeptical about the SEO of a 1=>2 page

What else would you include in it?

@lucavenir
Copy link
Contributor

If I'm not mistaken, with v2 we obtained:

  1. Notifier and AsyncNotifier
  2. A different behavior when using AsyncValue.when: you still have the previous value or error when AsyncValue is loading or refreshing
  3. The whole set of utilities connected to the above, such valueOrNull, hasError, hasData, asError, isLoading, etc.
  4. Any other breaking change I might miss in here (I recall that ProviderListener was removed, for instance)

Overall it was a big change that justifies the deprecation of StateNotifierProvider and then StateProvider also.

@lucavenir
Copy link
Contributor

Hey there!

Can I try and take care of this?

Also, let me know if this should be put into a broader topic, i.e. migrating from v1 to v2.

Once this is cleared out, I might add some commits based on what I read here.

@rrousselGit
Copy link
Owner Author

@lucavenir if you want there's also a bunch of documents on the rework-flow branch that would be worth starting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-rework Internal label for documentation-related work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants