flutter / devtools

Performance tools for Flutter
https://flutter.dev/docs/development/tools/devtools/
BSD 3-Clause "New" or "Revised" License
1.56k stars 321 forks source link

No need to show render properties in the inspector panel #8158

Open elliette opened 1 month ago

elliette commented 1 month ago

We decided to show the render properties along with the widget properties in the inspector panel because we didn't think the inspected widget was dumped to the console in IDEs like it is for DevTools. However, at least in VS Code the inspected widget is dumped to the eval console on widget selection, and the render object is available there:

Screenshot 2024-08-06 at 8 47 40 AM

Once I confirm that the same is true for IntelliJ, I think it makes sense to remove render properties from the inspector panel since we aren't showing new information.

elliette commented 1 month ago

@helin24 @jwren - I just tried clicking a widget in the Widget Tree to see if it gets dumped to the console in IntelliJ like it does for VS Code and Flutter DevTools but I don't see it there. Do you know if this is supported in IntelliJ?

helin24 commented 1 month ago

I think being written to the console in VS Code is from DAP, so it's not supported in IntelliJ. I thought for some reason this was a side effect rather than an intended action, but I may be confusing this with something else. @DanTup do you recall?

DanTup commented 1 month ago

The behaviour of inspected (via widget inspector) variables appearing in the debug console in VS Code is not intended, but is a long-standing bug because of the use of inspect().

The widget inspector called inspect() to trigger the notification over the VM Service that the IDEs listened for. However, the documentation for inspect is not very clear and makes it seem like users can call inspect() to see things in the debugger, so we had requests to make this work.

So, VS Code started listening for inspect calls and dumping them to the console. We tried to exclude things from the inspector (because I don't - or didn't - think anybody wanted this). Our ultimately fix was to move inspector off inspect() and using ToolEvents. However for compatibility, I think it still sends inspect() (the goal was to remove that once we're happy enough users are one IDEs that are handling the ToolEvent).

(we can ofc revisit this if we're not sure it's the right way - but it does feel a bit spammy to keep dumping to the debug console when clicking around the inspector IMO)

helin24 commented 1 month ago

Ah okay, thanks for that background info. So if we are to transition to using DTD and registering a navigate to code service method, then will we no longer have this? I suppose the ToolEvent will still be used for when someone uses the inspect button and clicks within the app, but perhaps only in-app clicks would result in variables appearing, while DevTools inspector clicks would not.

elliette commented 1 month ago

Ok, thanks for the context here! I think in that case we should keep the render properties, and potentially even change it to the evaluated variable. That way users still have access to this information (if they want it) but eventually without it spamming their console.

DanTup commented 1 month ago

So if we are to transition to using DTD and registering a navigate to code service method, then will we no longer have this?

Yeah, although we already have a ToolEvent that currently goes over the VM Service which VS Code supports (not sure about IntelliJ, but I thought so?), so we could presumably still get off the old inspect() if nobody else is using it (does DevTools?).

I suppose the ToolEvent will still be used for when someone uses the inspect button and clicks within the app, but perhaps only in-app clicks would result in variables appearing, while DevTools inspector clicks would not.

Not sure I understand... My understanding is that the toolEvent (currently sent over the VM Service, but I think we will also have a DTD version) was to be used for Flutter/inspector to trigger navigation in the IDE and we could stop Flutter/inspector using inspect for this (so the only code calling inspect() is using code, and it always shows the variable in the debug console).

helin24 commented 1 month ago

Sorry, I got confused. When a user is inspecting the app and clicking within the app to select widgets, that sends a EventKind.Inspect over VM service, not ToolEvent. I don't know how VS Code responds to those events currently, but I think it's a separate issue.

DanTup commented 1 month ago

Yeah, it's that Inspect event that VS Code/DAP listens to and prints the inspected variable (this is what is generated if a user calls inspect() in their code).

I think the initial issue is that the Inspect event was not well defined. The inspector was using, but users also wanted to use it to dump variables for their own debugging. Based on https://github.com/flutter/devtools/issues/2522#issuecomment-1652532465 I understood the goal was to stop Flutter from using it and instead use a custom event?