fix(core): prevent unnecessary node re-renders during canvas zoom#2398
fix(core): prevent unnecessary node re-renders during canvas zoom#2398cyphercodes wants to merge 1 commit intodidi:masterfrom
Conversation
Remove SCALE_X from destructuring in BaseNode render() to prevent mobx from tracking it as a reactive dependency. This fixes the performance issue where all nodes re-render when zooming the canvas. Closes didi#2396
|
|
Thank you for submitting this pull request and proposing a solution to the issue. However, after reviewing the change, I believe it only addresses the re-render problem when the conditional statement is not triggered. When the node is draggable (i.e., when the condition is satisfied), a re-render still occurs during canvas zoom operations. I believe a better approach would be to move LogicFlow/packages/core/src/tool/MultipleSelectTool.tsx Lines 36 to 37 in d661666 LogicFlow/packages/core/src/view/overlay/CanvasOverlay.tsx Lines 184 to 185 in d661666 Please note that this alternative solution has not been thoroughly tested yet. Additionally, I do not use |
There was a problem hiding this comment.
Pull request overview
Optimizes node view rendering during canvas zoom by attempting to prevent MobX from tracking transformModel.SCALE_X as a reactive dependency in BaseNode.render(), addressing performance regression reported in #2396.
Changes:
- Stop destructuring
SCALE_XfromgraphModel.transformModelinBaseNode.render(). - Update
stepDrag.setStep()to usetransformModel.SCALE_Xinstead of a destructuredSCALE_Xvariable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (adjustNodePosition && draggable) { | ||
| this.stepDrag.setStep(gridSize * SCALE_X) | ||
| this.stepDrag.setStep(gridSize * transformModel.SCALE_X) | ||
| } |
There was a problem hiding this comment.
This still reads the observable transformModel.SCALE_X during render(), so an observer-wrapped node view will continue to track SCALE_X and re-render on every zoom change. Switching from destructuring to transformModel.SCALE_X doesn’t avoid MobX dependency tracking; it only changes when the read occurs.
To actually prevent zoom-triggered node re-renders, avoid reading SCALE_X in render() entirely (e.g., move setStep() into handleMouseDown right before stepDrag.handleMouseDown, or wrap the read in mobx.untracked).
Description
Fixes #2396
When zooming the canvas, all nodes were unnecessarily re-rendering due to
SCALE_Xbeing destructured from the observabletransformModelin therender()method. This caused mobx to trackSCALE_Xas a reactive dependency, triggering re-renders for every node whenever the zoom level changed.Changes
SCALE_Xin render() to accessing it directly viatransformModel.SCALE_Xwhen neededSCALE_Xat the component levelImpact
Testing
setStep()is still called with the correct scaled grid size