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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant