Nested ProviderScope fix breaks family providers(?)

[dainnilsson]

Describe the bug

Commit fea8a96 seems to have broken a lot of providers in our app.

For now I can say that it looks like rebuilds aren't being triggered for ConsumerState instances that are using ref.watch() in their build methods even though the watched provider is changed.

In this instance, the underlying provider is a StateNotifierProvider with autoDispose and family, where we are switching the family parameter as we switch pages in the app. The StateNotifier holds a List<DataItem>? that is initially null, but later updated with a value. Only the Widget on the initial "page" shows list items. Once we switch to a new page (using a new family argument) the new Widget only shows the provided value as being null, even though it does get updated once data is fetched (which should trigger a rebuild).

Sorry for the vagueness of this report, I'm still investigating and will provide more details if I figure anything out.

To Reproduce

<Please add a small sample to that can be executed to reproduce the problem. As a general rule, 100 lines is the maximum>

Expected behavior
ConsumerState should rebuild when watched provider changes.

[dainnilsson]

I managed to create a small sample reproducing this. Run this app, click on the buttons to switch page. The first switch to a new page works fine, but once you go back to a previous page things quickly break and stop updating.

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

void main() {
  runApp(const MyApp());
}

class MyStateNotifier extends StateNotifier<List<String>?> {
  final String name;
  late Timer _timer;
  MyStateNotifier(this.name) : super(null) {
    _timer = Timer.periodic(const Duration(seconds: 1), (_) {
      final old = state ?? [];
      print("Updating $name");
      state = List.unmodifiable([...old, '$name ${old.length}']);
    });
  }

  @override
  void dispose() {
    print("Dispose $name");
    _timer.cancel();
    super.dispose();
  }
}

final myPage = StateProvider<String>((ref) => 'page 1');

final myProvider = StateNotifierProvider.autoDispose
    .family<MyStateNotifier, List<String>?, String>(
  (ref, arg) {
    throw UnimplementedError();
  },
);

final myOverriddenProvider = StateNotifierProvider.autoDispose
    .family<MyStateNotifier, List<String>?, String>(
  (ref, arg) {
    return MyStateNotifier(arg);
  },
);

class MyWidget extends ConsumerWidget {
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final page = ref.watch(myPage);
    List<String>? items = ref.watch(myProvider(page));

    if (items == null) {
      return const CircularProgressIndicator();
    }
    return Column(
      children: items.map((e) => Text(e)).toList(),
    );
  }
}

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

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
        title: 'Flutter Demo',
        theme: ThemeData(
          primarySwatch: Colors.blue,
        ),
        home: ProviderScope(
          overrides: [
            myProvider.overrideWithProvider(
                (argument) => myOverriddenProvider(argument))
          ],
          child: Scaffold(
            body: Column(
              children: [
                Consumer(builder: (context, ref, _) {
                  return Row(
                    children: ["Page 1", "Page 2", "Page 3"]
                        .map((e) => TextButton(
                              onPressed: () {
                                ref.read(myPage.notifier).state = e;
                              },
                              child: Text(e),
                            ))
                        .toList(),
                  );
                }),
                MyWidget(),
              ],
            ),
          ),
        ));
  }
}

Doing the same with Riverpod 2.3.3 works fine.

EDIT: Note the provider override! Without that, thing seem to work.

[jeiea]

I think it's regression. The following test would suffice.

  test(
      'when cancel autoDispose family override it should be disposed immediately',
      () async {
    final provider = Provider.autoDispose.family<int, int>((ref, _) => -1);

    var constructionCount = 0;
    final override = Provider.autoDispose.family<int, int>((ref, _) {
      return ++constructionCount;
    });

    final root = createContainer();
    final container = createContainer(
      parent: root,
      overrides: [provider.overrideWithProvider(override)],
    );

    var count = container.read(provider(0));
    expect(count, 1);
    await container.pump();

    count = container.read(provider(0));
    expect(count, 2);
  });

[jeiea]

Can you test if the above PR resolves the issue?

[dainnilsson]

I've now done some testing of the PR and from what I can tell, it fixes the problem!

[rrousselGit]

Oh, I didn't realize that fea8a96 changed the if condition.

I was confused why the proposed PR was related to the problem, but it seems to simply revert the involuntary condition change. So LGTM.
Will merge in a bit