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

feat: access to defined scope #17

Closed
wants to merge 9 commits into from
Closed

feat: access to defined scope #17

wants to merge 9 commits into from

Conversation

nank1ro
Copy link
Member

@nank1ro nank1ro commented Feb 3, 2025

Allow providers to access the scope where they are defined.
This now it totally feasible

testWidgets('Test Provider.of within Provider create fn', (tester) async {
    final numberProvider = Provider((_) => 5);

    final doubleNumberProvider = Provider((context) {
      final number = numberProvider.of(context);
      return number * 2;
    });

    await tester.pumpWidget(
      MaterialApp(
        home: Scaffold(
          body: ProviderScope(
            providers: [numberProvider, doubleNumberProvider],
            child: Builder(
              builder: (context) {
                final number = numberProvider.of(context);
                final doubleNumber = doubleNumberProvider.of(context);
                return Text('$number $doubleNumber');
              },
            ),
          ),
        ),
      ),
    );
    Finder numberFinder(int value1, int value2) => find.text('$value1 $value2');
    expect(numberFinder(5, 10), findsOneWidget);
  });

This removes the boilerplate of using two nested ProviderScope widgets

Copy link

Deploying flutter-disco with  Cloudflare Pages  Cloudflare Pages

Latest commit: d6c4392
Status: ✅  Deploy successful!
Preview URL: https://1aceb0b9.flutter-disco.pages.dev
Branch Preview URL: https://feat-access-to-scope.flutter-disco.pages.dev

View logs

Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #17   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          215       215           
=========================================
  Hits           215       215           
Files with missing lines Coverage Δ
...ib/src/models/providers/instantiable_provider.dart 100.00% <ø> (ø)
...kages/disco/lib/src/models/providers/provider.dart 100.00% <100.00%> (ø)
...co/lib/src/models/providers/provider_argument.dart 100.00% <100.00%> (ø)
packages/disco/lib/src/widgets/provider_scope.dart 100.00% <100.00%> (ø)

Comment on lines 118 to 121
// cannot be late
// ignore: use_late_for_private_fields_and_variables
ProviderScopeState? _scopeState;

Copy link
Member

@manuel-plavsic manuel-plavsic Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like the ProviderScopeState? _scopeState that is here (and the one in ArgProvider). Can we do this differently, somehow? I am confident we should keep our providers immutable.

I am pretty sure the ProviderScopeState? _scopeState introduces bugs. Remember the one we had with ArgProvider when replacing keys? I am fearing something similar here...

ProviderScope should handle the logic. If it is not possible to move it there, then I would rather close this without merging.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I didn't know about deactivate.
Before the problem, when the key of ProviderScope change, was that the dispose was called after initState. But deactivate is called before.

I think I can refactor the business logic again (in another PR) and make it working removing many models and keeping the logic pretty simple.

Copy link
Member Author

@nank1ro nank1ro Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The immutability bring no value here. There are just variables with a value assigned, they are still final. (Immutable)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: all tests are passing, including the key change which I tested manually

@nank1ro
Copy link
Member Author

nank1ro commented Feb 15, 2025

💩

@nank1ro nank1ro closed this Feb 15, 2025
@nank1ro nank1ro deleted the feat/access-to-scope branch February 20, 2025 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants