fix(vscode): Adding a step in Kaoto VSCode takes several seconds #1338
Conversation
| @@ -25,8 +25,6 @@ const Visualization = () => { | |||
| zoom: 1.2, | |||
| }; | |||
| const [isPanelExpanded, setIsPanelExpanded] = useState(false); | |||
| const [, setReactFlowInstance] = useState(null); | |||
There was a problem hiding this comment.
@kahboom is this needed? I think that it forces a render but I don't know if there another use case for it 🤔
There was a problem hiding this comment.
confession time--I have no idea. I think that might have been from an older version of React Flow, but I can't be sure. 😅 We can remove it, just be sure to check rendering still happens as needed with the basic actions (for example replacing from big catalog, deleting steps, editing config, etc.)
There was a problem hiding this comment.
Mmm, I think that you're right, I checked here and it seems that it's also deprecated.
| > | ||
| <div | ||
| className="reactflow-wrapper" | ||
| data-testid={'react-flow-wrapper'} | ||
| ref={reactFlowWrapper} | ||
| style={{ |
There was a problem hiding this comment.
Moved to the corresponding Visualization.css file
Codecov Report
@@ Coverage Diff @@
## main #1338 +/- ##
==========================================
- Coverage 52.93% 52.88% -0.05%
==========================================
Files 56 56
Lines 1910 1908 -2
Branches 435 435
==========================================
- Hits 1011 1009 -2
Misses 851 851
Partials 48 48
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -40,7 +38,10 @@ const Visualization = () => { | |||
| const previousLayout = useRef(visualizationStore.layout); | |||
| const previousIntegrationJson = useRef(integrationJsonStore.integrationJson); | |||
| const visualizationService = new VisualizationService(integrationJsonStore, visualizationStore); | |||
| const stepsService = new StepsService(integrationJsonStore, nestedStepsStore, visualizationStore); | |||
| const stepsService = useMemo( | |||
There was a problem hiding this comment.
👀 just curious, should useMemo() be the default way of instantiating (and caching) service instances? a "memo" for #1233
There was a problem hiding this comment.
That's a very interesting question 😃
From my point of view, I don't think that this is the definite way to go but a transition point.
At first, the onNodeClick function was being recreated on every render pass, together with the stepService, so as I wrapped onNodeClick function into a useCallback() hook to avoid recreating it, stepService now it's a required dependency of it, so I wrapped the service with useMemo() to recreate it only if something change.
I think that a more appropriate fix would be to have this service as a context, this way we wouldn't need to create the service in different places but just call the useContext() hook where needed.
There was a problem hiding this comment.
aha! makes sense, thanks 👍
There was a problem hiding this comment.
@lordrip It looks good to me. Did some basic testing locally. Will wait on you before merging, since I'm not sure if you still wanted to remove the setReactFlowInstance code, particularly if it is causing an extra render we don't need.
Yes, it seems that |
Currently, adding a new step from the `VSCode` extension takes more than 12 seconds. This is because, in `VSCode`, the `MiniCatalog` component gets rerendered many times blocking the UI thread. Part of the solution is to avoid recreating functions inside of components that later on are passed to child components. A more robust solution would include removing many other points that trigger extra renders. Changes * Move the `onNodeClick` function inside of a `useCallback()` hook * Given that stepsService it's a dependency of `onNodeClick`, it needed to be moved inside a `useMemo()` hook. * Remove `setReactFlowInstance` since seems not used * Remove `reactFlowWrapper` since is not linked with anything * Move inline styles to a dedicated CSS class Fixes KaotoIO/vscode-kaoto#132
|
Kudos, SonarCloud Quality Gate passed!
|
Description
Currently, adding a new step from the
VSCodeextension takes more than 12 seconds.This is because, in
VSCode, theMiniCatalogcomponent gets rerendered many times blocking the UI thread.Examples
KaotoUI - Firefox Developer edition
KaotoUI - VSCode
Notice how in VSCode there are many more render passes hence blocking the UI thread.
Part of the solution is to avoid recreating functions inside of components that later on are passed to child components. A more robust solution would include removing many other points that trigger extra renders.
Changes
onNodeClickfunction inside of auseCallback()hookonNodeClick, it needed to be moved inside auseMemo()hook.setReactFlowInstancesince seems not usedreactFlowWrappersince is not linked with anythingFixes KaotoIO/vscode-kaoto#132