fix(tree): avoid crashes when data is emptied or nodes are removed. - #21735
fix(tree): avoid crashes when data is emptied or nodes are removed.#21735SEPURI-SAI-KRISHNA wants to merge 1 commit into
Conversation
|
Thanks for your contribution! Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only. |
|
Noting some prior work I came across: the The other change here is independent of #18491: when If you'd rather land #18491 for the |
Brief Information
This pull request is in the type of:
What does this PR do?
Fixes two crashes in the tree series: rendering a tree with no laid-out nodes, and removing several nodes in one update.
Fixed issues
Details
Before: What was the problem?
Two independent crashes, both hit by ordinary tree data updates.
1. A tree with no nodes to lay out throws.
_updateViewCoordSyscollects node positions and computes their extent:bbox.fromPointsreturns immediately without writing anything whenpointsisempty, so
minandmaxstay empty arrays.max[0] - min[0]is thenNaN, theexisting zero-size corrections below only test
=== 0so they never fire, and thedataRect handed to the view coordinate system is
NaN. That makes the resultingmatrix non-invertible,
matrix.invertreturnsnull, andlegacyCopyOverallTransdereferences it.
This is not only the literal
data: []case — it also covers a tree whose nodeshave no valid layout yet, e.g. rendering before an async fetch resolves, or after
filtering the data down to nothing.
2. Removing several nodes in one update throws.
removeNodeEdgeguards its own node's graphic element but not its source's:removeNodesetsdata.setItemGraphicEl(dataIndex, null)in the removal callback.With
animation: falsethat callback runs synchronously, so when a batch of nodesis removed the source node's element is frequently already
nullby the time itschildren's edges are cleaned up.
This one is independent of the empty-data case: it fires whenever removed nodes
include a parent, even when the resulting tree is not empty (dropping a subtree, or
collapsing back to just the root).
After: How does it behave after the fixing?
min/maxare seeded when there is no valid point — from the previous extent ifthere is one (the mechanism already used for the collapsed-root case), otherwise
from zero. The existing zero-size corrections then expand it into a usable rect,
so the view transform stays invertible.
sourceSymbolElis guarded exactly likesymbolElimmediately above it.sourceEdgeis only used as a fallback and everything downstream is alreadybehind
if (edge), so a missing source simply means there is no edge to remove.Emptying a tree, refilling it, and removing subtrees all work with animation on and off.
Document Info
One of the following should be checked.
Misc
Security Checking
ZRender Changes
Related test cases or examples to use the new APIs
Added
test/ut/spec/series/treeUpdate.test.ts, covering an initially empty tree, atree emptied after having data, removing a subtree, removing all but the root, and
refilling an empty tree — with
animationbothfalseandtrue.On
masterthe empty-data cases fail regardless of animation, and the node-removalcases fail with
animation: false. Theanimation: trueremoval cases pass beforeand after, and are kept to document that the timing of the removal callback is what
exposes the second bug.
npm run test,npx tsc --noEmitandeslinton the changed file all pass.Merging options
Other information
src/chart/tree/TreeView.tsis also touched by #18491, #21603 and #21681.#18491 is in the same function as the second guard here and fixes the same crash
(more thoroughly — it threads the symbols through so
sourceSymbolElis nevernull, and also fixes polyline edge cleanup when animation is off). If you would
rather land that one, I am happy to drop the
removeNodeEdgehunk and scope thisPR to the empty-data crash, which #18491 does not cover. #21603 and #21681 are in
different functions; whichever lands first may need a trivial rebase.
Both crashes end in the same place from a user's point of view — clearing or
shrinking tree data — so they are fixed together rather than split across two PRs
that would conflict in the same file.