Pass 'valueId' (Diagnosticable) instead of 'objectId' (DiagnosticsNode) to inspector API.#5918
Pass 'valueId' (Diagnosticable) instead of 'objectId' (DiagnosticsNode) to inspector API.#5918polina-c merged 11 commits intoflutter:masterfrom
Conversation
This reverts commit 0c7f14b.
polina-c
changed the title
polina-c
changed the title
| _refreshRateLimiter.scheduleRequest(); | ||
| } | ||
|
|
||
| bool identicalDiagnosticsNodes( |
There was a problem hiding this comment.
Fyi @incendial, why wasn't this method detected as dead code?
There was a problem hiding this comment.
Good question, will check.
There was a problem hiding this comment.
I think I was running the command without the --monorepo option. And since inspector_controller is in the package exports, any unused code is skipped.
With the option it shows a bunch of things:
lib/src/screens/inspector/inspector_controller.dart:
⚠ unused field highlightNodesShownInBothTrees
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:237:3
⚠ unused method getTreeType
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:254:3
⚠ unused method highlightShowFromNodeInstanceRef
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:285:3
⚠ unused method getSubtreeRootValue
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:493:3
⚠ unused method getTreeNode
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:574:3
⚠ unused method maybeUpdateValueUI
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:578:3
⚠ unused method identicalDiagnosticsNodes
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:602:3
⚠ unused method treeTypeDisplayName
at /Users/dimannich/Desktop/code/devtools/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart:922:3
There was a problem hiding this comment.
And in other files as well.
Can create a PR if the team is ready for another unused code removal round 🙂.
There was a problem hiding this comment.
It would be amazing. Thank you!!!
There was a problem hiding this comment.
Actually, I was running the command on a stale fork, so some of these reports have already been removed 😅
|
|
||
| void updateHighlighted(L? newProperties); | ||
|
|
||
| String? id(RemoteDiagnosticsNode? node) => node?.dartDiagnosticRef.id; |
There was a problem hiding this comment.
Make sure to verify that the layout explorer still works after this change. For example, try changing flex values for widgets in a row.
There was a problem hiding this comment.
Screen.Recording.2023-06-16.at.1.47.59.PM.mov
jacob314
left a comment
jacob314
left a comment
There was a problem hiding this comment.
Thank you for cleaning up this tech debt! This should make the inspector more performant (half as many object ids created) as well as leak less memory.
…sticsNode) to inspector API. (flutter#5918)" This reverts commit 3ce878c.
3 participants
Prerequisite PRs that update APIs to take Diagnosticable:
flutter/flutter#128833
getChildrenSummaryTree
flutter/flutter#128897
getProperties
flutter/flutter#128962
getSelectedSummaryWidget
getSelectedWidget
getChildrenSummaryTree
getChildrenDetailsSubtree
getDetailsSubtree
getLayoutExplorerNode
Tests should start passing after switching to this version of Flutter: