Skip to content

fix(model): guard against null data in getDataParams and event handler#21537

Open
davesecops wants to merge 1 commit intoapache:masterfrom
davesecops:fix-21535
Open

fix(model): guard against null data in getDataParams and event handler#21537
davesecops wants to merge 1 commit intoapache:masterfrom
davesecops:fix-21535

Conversation

@davesecops
Copy link

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Adds null guards to getDataParams() and its 6 chart-specific overrides to prevent TypeErrors when getData() returns null on disposed series during mouse events.

Fixed issues

Details

Before: What was the problem?

When a chart series is disposed (via echarts.dispose(), component unmount in React/Vue/Angular, setOption with notMerge, or toolbox restore), SeriesModel.getData() returns null/undefined. The comment in Series.ts explicitly acknowledges this:

// When series is not alive (that may happen when click toolbox // restore or setOption with not merge mode), series data may // be still need to judge animation or something when graphic // elements want to know whether fade out. return inner(this).data; // ← can be undefined

However, getDataParams() and its overrides unconditionally call methods on the result:

// dataFormat.ts — base method const data = this.getData(dataType); const rawDataIndex = data.getRawIndex(dataIndex); // 💥 TypeError // TreemapSeries.ts — override const node = this.getData().tree.getNodeByDataIndex(dataIndex); // 💥 TypeError // PieSeries.ts — override const data = this.getData(); data.each(data.mapDimension('value'), ...); // 💥 TypeError

The primary crash trigger is the event handler in echarts.ts which calls getDataParams() during mousemove/mouseout events that fire on stale DOM elements after chart disposal. This affects all chart types in all frameworks where component lifecycle causes disposal during user interaction.

After: How does it behave after the fixing?

Three layers of defense:

1. Base method guard (src/model/mixin/dataFormat.ts):

const data = this.getData(dataType); if (!data) { return {} as CallbackDataParams; }

Returns {} when data is null — consistent with the existing fallback at echarts.ts:778 (dataModel.getDataParams(...) || {}). Protects all callers outside the event handler (tooltips, labels, visual pipeline).

2. Event handler guard (src/core/echarts.ts):

try { params = (dataModel && dataModel.getDataParams(...) || {}) as ECElementEvent; } catch (e) { params = {} as ECElementEvent; }

Wraps the highest-risk call site in try/catch, catching crashes from any chart override's getData() access.

3. Override guards (6 chart series files):
Each override now checks getData() before accessing chart-specific properties:

  • TreemapSeries.ts — guards data.tree access
  • SunburstSeries.ts — guards data.tree access
  • TreeSeries.ts — guards data.tree access
  • PieSeries.ts — guards data.each(), data.mapDimension() access
  • FunnelSeries.ts — guards data.mapDimension(), data.getSum() access
  • ChordSeries.ts — guards data.getName(), this.getGraph() access

This pattern follows existing precedent in the codebase — SeriesModel.getAllData() already null-guards getData():

const mainData = this.getData(); return mainData && mainData.getLinkedDataAll ? mainData.getLinkedDataAll() : [{ data: mainData }];

Related test cases

Added test/ut/spec/model/getDataParams.test.ts — 5 test cases covering pie, treemap, funnel, sunburst, and tree series. Each test mocks getData() to return null and verifies getDataParams() does not throw.

All existing tests continue to pass (the 1 pre-existing failure in time.test.ts:roundTime_locale is a DST-dependent issue on master, unrelated to this PR).

Document Info

  • This PR doesn't relate to document changes

Misc

Security Checking

  • This PR uses security-sensitive Web APIs.

ZRender Changes

  • This PR depends on ZRender changes.

Related test cases or examples to use the new APIs

See test/ut/spec/model/getDataParams.test.ts — 5 unit tests covering all guarded chart types.

Merging options

  • Please squash the commits into a single one when merging.

Other information

This bug has been open since 2018 (#9402) and affects all chart types. It is the most commonly reported ECharts crash in React/Vue applications where charts are rendered in routed views. The fix is minimal (35 lines added across 8 source files + 1 test file), safe (returns empty params matching existing fallback patterns), and follows internal precedent.

@echarts-bot
Copy link

echarts-bot bot commented Mar 11, 2026

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment