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

Mitigate ProviderNotFoundException with context redirection #862

Open
arthurbcd opened this issue Feb 23, 2024 · 7 comments
Open

Mitigate ProviderNotFoundException with context redirection #862

arthurbcd opened this issue Feb 23, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@arthurbcd
Copy link

arthurbcd commented Feb 23, 2024

Is your feature request related to a problem? Please describe.

  • Problem: Difficulty accessing providers in new contexts created by dialogs, modals, pushed routes, leading to frustration and increased boilerplate.
  • Example: Unable to access a set of providers within showDialog without manual data passing or restructuring the widget tree.
void main() => runApp(
      Provider(
        create: (context) => 'Hello, World!',
        child: const MaterialApp(home: Center(child: Example())),
      ),
    );

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

  @override
  Widget build(BuildContext context) {
    // CAN access the text
    final hello = Provider.of<String>();

    return ElevatedButton(
      onPressed: () {
        showDialog(
          context: context,
          builder: (_) => const HelloDialog(),
        );
      },
      child: Text('Say: $hello'),
    );
  }
}

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

  @override
  Widget build(BuildContext context) {
    // CANNOT access the text
    final hello = Provider.of<String>(context);

    return AlertDialog(title: Text(hello));
  }
}

Describe the solution you'd like

  • Feature: A context redirect mechanism in the Provider package.
  • How it works: Make Provider.of to check for an "inherited context" if the current context lacks the requested provider, allowing widgets to access providers from a parent context seamlessly.
  • Implementation: Introduce a way to specify an "inherited context" when creating new widget tree branches, ensuring provider accessibility across different contexts.

Proposal

We simply provide the "redirecting context".

It's actually intuitive: "can't find the correct provider above context? then provide the correct context"

showDialog(
  context: context,
  builder: (_) => Provider.value(
    value: context,
    child: const HelloDialog(),
  ),
);

Or a new dedicated InheritedWidget. Which would not break projects that actually use the above case.

This way, we wouldn't be able to context.read<BuildContext>(), discouraging it. Only the library would use.

showDialog(
 context: context,
 builder: (_) => Provider.redirect( // we can refine this api
   context: context,
   child: const HelloDialog(),
 ),
);

Internally, provider would first check for a "redirecting context", and use it instead of throwing.

//  try look for the redirecting context before throwing everyone's favorite exception
final inheritedElement = //...

if (inheritedElement == null && null is! T) {
 throw ProviderNotFoundException(T, context.widget.runtimeType);
}

Now, finally.

 Widget build(BuildContext context) {
   // CAN access the text
   final hello = Provider.of<String>(context);

   return AlertDialog(title: Text(hello));
 }

Describe alternatives you've considered

  • Manual Data Passing: Increases boilerplate, disables const widgets, decreases readability.
  • Global Access Patterns: Leads to less maintainable code.
  • Widget Tree Restructuring: Often impractical due to complexity.

All above are even worse if we consider pushed pages that require many different providers. Redirecting the context is scalable, simple and efficient.

Additional context

This proposal addresses a significant pain point in provider. Simplifying provider access across contexts will empower developers to build complex applications more effectively, reducing the need for workarounds and fostering best practices.

@caseycrogers
Copy link

Interesting idea! Would certainly be helpful.

Usually what I do is I grab references to the necessary context-dependent state objects (ThemeData etc) and then pass them directly into the modal or re-wrap the modal with the respective inherited widgets. This would include providers where applicable.

I think the biggest challenge with your above example is if the inherited context goes stale you'll no longer be able to inherit fro it so you either:

  1. Silently stop referencing providers from that context-confusing and likely to lead back to provider not found etc
  2. Loudly fail, but now the whole system is going to be quite brittle

Do you have thoughts for addressing this downside? Perhaps the answer is "for this use-pattern it's not a problem, don't use it".

@rrousselGit
Copy link
Owner

Yes the context going stale issue is a big one.

It's particularly problematic when using things like navigator.pushReplacement

@rrousselGit
Copy link
Owner

Note that Riverpod already offers such a feature. In a way that's neater at that. Rather than having to list all providers by hand, you can tell ProviderScope to import all providers from the previous route.

The context going stale issue is still one. But it could be solved in Riverpod specifically, as it isn't bound to the BuildContext. I just haven't done it yet

@arthurbcd
Copy link
Author

arthurbcd commented Feb 23, 2024

I agree with @caseycrogers. I would see the stale problem as expected error. It's to the developer to choose which context he wants to wire in.

The stale problem could be handled the same way Flutter already handles for it's own inherited widgets:

Future<T?> showDialog<T>({
...
}) {
  assert(_debugIsActive(context)); // we can do it also
  assert(debugCheckHasMaterialLocalizations(context));

  final CapturedThemes themes = InheritedTheme.capture(
    from: context,
    to: Navigator.of(
      context,
      rootNavigator: useRootNavigator,
    ).context, // <- flutter provides another "redirecting context"
  );

As for the navigator.pushReplacement:

showDialog(
  context: context,
  builder: (_) => Provider.redirect(
    context: Navigator.of(context).context, // the developer provides a context that won't be stale
    child: const HelloDialog(),
  ),
);

Of course we can always improve documentation and asserts to improve DX.

But still, for most use cases: showDialog, showModelBottomSheet, etc. The current context would just be fine.

Thanks for the considerations @caseycrogers, @rrousselGit.

@rrousselGit
Copy link
Owner

The stale problem is different for provider vs Flutter's theme.

Provider values are often disposable. And once disposed, a notifier cannot be used anymore.
Whereas a ThemeData is a purely immutable object. It can freely be used.

@clragon
Copy link

clragon commented Feb 27, 2024

The logical solution to this, in my opinion, would be to use a nested navigator to provide your values accross all the pages that you need to have those Providers in. I haven't however found any satisifable solution that just lets me create new nested navigators that act completely transparent (meaning, navigating in either direction just feels like a single navigator).

@arthurbcd
Copy link
Author

@clragon

The Navigator is a smart solution, as the InheritedWidget will be scoped to the it.

Thats what I actually do, making it possible to easily scope providers and access them (even on modals). You would depend on go_router thought.

Code:
https://github.com/arthurbcd/go_provider/blob/main/lib/go_provider.dart

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants