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

fix: not disposing autoDispose family provider #2481

Merged
merged 6 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
matrix:
channel:
- stable
- master
package_path:
- examples/counter
- examples/marvel
Expand Down Expand Up @@ -70,10 +71,11 @@ jobs:

- name: Check format
run: dart format --set-exit-if-changed .
if: matrix.package_path != 'website'
if: matrix.package_path != 'website' && matrix.channel == 'master'

- name: Analyze
run: flutter analyze
if: matrix.channel == 'master'

- name: Run tests
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ class AutoDisposeChangeNotifierProviderFamily<NotifierT extends ChangeNotifier?,
NotifierT Function(
AutoDisposeChangeNotifierProviderRef<NotifierT> ref,
Arg arg,
)
create,
) create,
) {
return FamilyOverrideImpl<NotifierT, Arg,
AutoDisposeChangeNotifierProvider<NotifierT>>(
Expand Down
1 change: 1 addition & 0 deletions packages/riverpod/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Improved error message for missing `dependencies` (thanks to @ValentinVignal)
- Fixed various typos in the documentation (thanks to @ValentinVignal)
- Fix not disposing autoDispose family providers

## 2.3.5 - 2023-04-18

Expand Down
3 changes: 1 addition & 2 deletions packages/riverpod/lib/src/async_notifier/base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,7 @@ mixin FutureHandlerProviderElementMixin<T>
required void Function(Object, StackTrace) error,
required void Function() done,
required void Function(Future<T>, CancelAsyncSubscription) last,
})
listen, {
}) listen, {
required bool didChangeDependency,
}) {
_onLoading(AsyncLoading<T>(), seamless: !didChangeDependency);
Expand Down
3 changes: 1 addition & 2 deletions packages/riverpod/lib/src/framework/container.dart
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,7 @@ class ProviderContainer implements Node {
// on provider dispose, to avoid memory leak

void removeStateReaderFrom(ProviderContainer container) {
final reader = container._stateReaders[element._origin];
if (reader?.override == provider) {
if (container._stateReaders[element._origin] == reader) {
container._stateReaders.remove(element._origin);
}
container._children.forEach(removeStateReaderFrom);
Expand Down
6 changes: 3 additions & 3 deletions packages/riverpod/lib/src/framework/family.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ typedef FamilyCreate<T, R extends Ref, Arg> = T Function(R ref, Arg arg);
/// A base class for all families
abstract class Family<
@Deprecated(
'The generic parameter will be removed in version 3.0.0. '
'This is to enable riverpod_generator to implement families with generic parameters',
)
'The generic parameter will be removed in version 3.0.0. '
'This is to enable riverpod_generator to implement families with generic parameters',
)
// ignore: deprecated_member_use_from_same_package
State> implements FamilyOverride<State>, ProviderOrFamily {
/// A base class for all families
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ class AutoDisposeStateNotifierProviderFamily<NotifierT extends StateNotifier<T>,
NotifierT Function(
AutoDisposeStateNotifierProviderRef<NotifierT, T> ref,
Arg arg,
)
create,
) create,
) {
return FamilyOverrideImpl<T, Arg,
AutoDisposeStateNotifierProvider<NotifierT, T>>(
Expand Down
20 changes: 20 additions & 0 deletions packages/riverpod/test/framework/auto_dispose_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,26 @@ final alwaysAlive = Provider((ref) {
expect(container.getAllProviderElements(), isEmpty);
});

test('supports disposing of overridden families', () async {
// Regression test for https://github.com/rrousselGit/riverpod/issues/2480
final provider = Provider.autoDispose.family<int, int>((ref, _) => -1);
rrousselGit marked this conversation as resolved.
Show resolved Hide resolved

var constructionCount = 0;
final root = createContainer();
final container = createContainer(
parent: root,
overrides: [provider.overrideWith((ref, arg) => ++constructionCount)],
);

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

await container.pump();
rrousselGit marked this conversation as resolved.
Show resolved Hide resolved

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

test(
'can select auto-dispose providers if the selecting provider is auto-dispose too',
() {
Expand Down