riverpod
riverpod copied to clipboard
rrousselGit
Provider Hierarchy inspector
Similar to the state inspector #1033, we could have a tab in the Flutter devtool that shows the graph of providers/consumers
It could look like:
where the various nodes would be providers/consumers
This would be especially helpful for debugging why an "autoDispose" provider is not getting disposed
Hey @rrousselGit! Do you have any updates on devtools for riverpod?
I was reading your post, looking at the code at the devtools package and where the provider package sends the debug info to devtools, and I think that I might be able to help porting the same idea to riverpod.
My idea, initially, is to have the same functionality as the current Provider implementation and later try to evolve to this graph view (which I loved the idea, BTW).
Do you have any concerns about this? Maybe you've already explored this and found some limitations due to riverpod's architecture.
I see that for Provider you've added a listener overriding the InheritedElement mount method, then I was looking at riverpod's repo and found that you are already exposing riverpod:provider_changed (#356). Did you find any blocker after this?
PS: I love how you are already using riverpod on the devtools implementation for provider haha
Hello!
That's fine. That is typically what I planned to do. I'm just working on other things for now
Perfect, I'll start working on it then! I'll let you know when I have something to show.
hey @rrousselGit! I have a working POC here.
https://user-images.githubusercontent.com/11666470/174619354-bf21d513-4957-4ca5-b293-9684b82830d5.mp4
Just want to align the UI for this first part. I basically used the same components from Provider's implementation. The only difference is that riverpod has the concept of containers, so the left panel has to show the list of containers with their provider elements.
Also, as we only have access to the containers list, it was a bit hard to get the provider runtime type, so I had to use a eval that is a bit more complex:
final providerElementsInstance = await eval.evalInstance(
'['
' for (final element in RiverpodBinding.debugInstance.containers["$id"]?.getAllProviderElements().toList() ?? [])'
' {'
' "state": element.getState()?.stateOrNull,'
' "type": element.origin.runtimeType.toString(),'
' }'
']',
isAlive: isAlive,
);
This way, for the given container ID, we get the origin type and the state value.
WDYT about the UI and the eval approach? If we are good with them I can polish my code and open a PR.
Nice job!
About your query, I'd be careful about a few things:
-
getStatecan be out of date, such as if a provider isn't listened but one of its dependencies has changed. The UI likely needs an icon to inform users of this (and maybe offer them to force a refresh of the provider) -
You need to handle errors too. Using
stateOrNullmeans you're ignoring errors. If a provider throws, we do want to show the error/stacktrace instead of "null" -
We also want to show the "name" of a provider in the UI, instead of only the provider type when possible.
final foo = Provider(..., name: 'hello world');Once we have metaprogramming, all providers will have their names specified.
Perfect! Maybe for the error/state we could show the Result<T> directly. The only change would be one more level on the preview panel.
https://user-images.githubusercontent.com/11666470/174632911-555954d5-f625-44b7-af4c-2391298aa247.mp4
I just need to figure out the stacktrace 🤔 , it won't show using the same preview as in provider's implementation.
About the name it should be pretty easy, I'll try adding it here.
Now, about the out of date state, it might be a bit tricky. I tried adding a refresh here, but the eval always change the instance IDs, so I was having a hard time keeping the selected state on the right panel. But I think it is possible to have a combination of provider type + name for the selected key, this way it should work. I'm just concerned that this might not be enough to have a strong unique key.
About the provider name, what do you think of doing something like this?
The logic is basically:
final typeString = '${node.type}()';
final title = node.name != null ? '${node.name} - $typeString' : typeString;
Hey @rrousselGit, I'm wondering if this will be an issue:
getState can be out of date, such as if a provider isn't listened but one of its dependencies has changed. The UI likely needs an icon to inform users of this (and maybe offer them to force a refresh of the provider)
Won't the element be removed from its container when something like this happens? I'm asking this because, as the solution is currently implemented, it will only take the containers map value once, or every time the list changes when a 'riverpod:container_list_changed' event is triggered.
Or I'm understanding it wrong? Maybe when a Provider is not a autoDispose provider and was listened once and is not currently listened this could happen 🤔 , is that it?
Indeed.
But feel free to add new events to the binding class. I'd expect events for when elements are added/updated/removed
Perfect, I was trying to avoid changing the binding class, so we wouldn't need to have a min version for dev-tools to work. But I think it is better to change it, so we can avoid having a too complex eval.
All of this
final providerElementsInstance = await eval.evalInstance(
'['
' for (final element in RiverpodBinding.debugInstance.containers["$id"]?.getAllProviderElements().toList() ?? [])'
' {'
' "state": element.getState()?.stateOrNull,'
' "type": element.origin.runtimeType.toString(),'
' }'
']',
isAlive: isAlive,
);
could be moved to the binding class and we could expose a list of ContainerNode that have RiverpodNodes or something like that.
I'd add the current Riverpod version to the binding too
The devtool should then check that this is a supported version.
I'm trying to find the best place to put the refresh debug event. I found that ProviderBase has a _onListen and a _onRemoveListener methods. What do you think of putting the event there? This way we would get this case when a provider has no listeners. Also, the container instance has a refresh method, which could be used to refresh the states.
The only issue is to figure out how to call the refresh via eval, worst case we could refresh every provider in the given container.
So we can show this info to the user, so they can be aware that the value might not be what is being shown. And have a button so they can refresh it.
Or we can just show a warning for now and ignore the refresh, this would be way simpler.
The context is this comment that you've made:
getState can be out of date, such as if a provider isn't listened but one of its dependencies has changed. The UI likely needs an icon to inform users of this (and maybe offer them to force a refresh of the provider)
What I'm trying to say is that we need to notify the devtool when a provider has its listeners changed, so we can show some indicator that its value could be outdated.
When I say refresh event I'm meaning
debugPostEvent('riverpod:container_list_changed');
Or other debug event, so we can listen there on the devtool implementation.
I see
Then you can probably add the logic in mayNeedDispose/flush for respectively checking listener removal / listener addition
Perfect! Will do, soon I'll open a PR here and another there on devtool so we can discuss code!
https://user-images.githubusercontent.com/11666470/174911002-a28792d1-ebca-4c7c-b56e-b88002afdfea.mp4
@rrousselGit just sharing with you the final experience that I have in mind. For now I'll just add a warning to providers that have no listeners. Refreshing the provider state is out of scope, maybe if we see value on this we can add on another PR.
I still need to polish the code, when I have the PRs I'll post them here.
Nice!
Note that this warning sounds a bit too generic. Providers know if they are out of date (cf flush). We should be able to not render the warning unless the provider indeed is out of date
You mean this flush?
I didn't get how to know that the provider might be out of date. Is it the _mustRecomputeState flag?
Correct
Also mayNeedDispose may not be the correct lifecycle after all. I don't remember all the names from memory, but there's probably something more appropriate.
We'll see when you make a PR.
perfect! I'll use the _mustRecomputeState flag instead of hasListeners then!
hey @rrousselGit here is the first PR https://github.com/rrousselGit/riverpod/pull/1471
This is the simple one, the other for devtools is on the way, still got to finish polishing the code.
@rrousselGit just opened the devtools PR, as a draft, so we can have discussions over there.
https://github.com/flutter/devtools/pull/4210
@adsonpleal For now my Widget Inspector is unable to display the contents of Providers and I am developing it by outputting it to the console log. So I am looking forward to this Draft being merged so much! 💙 Any obstacles regarding it?
