Skip to content

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

Closed
davesecops wants to merge 1 commit intoapache:masterfrom
davesecops:fix/null-guard-getDataParams
Closed

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

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 }];

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

N/A — this is a defensive fix. A test case would require simulating chart disposal during mouse events (component unmount race condition).

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 files), safe (returns empty params matching existing fallback patterns), and follows internal precedent.

When a series is disposed (component unmount, setOption with notMerge,
toolbox restore), getData() can return null — as acknowledged by the
comment in SeriesModel.getData(). However, getDataParams() and its
overrides unconditionally access properties on the result, causing
TypeErrors when mouse events fire on stale DOM elements after disposal.

This fix adds null guards at three levels:

1. DataFormatMixin.getDataParams() base method — returns {} when
   getData() is null, protecting all callers (tooltips, labels, etc.)

2. Event handler in echarts.ts — wraps getDataParams() call in
   try/catch, protecting against crashes in any chart override

3. All 6 chart-specific getDataParams() overrides — each now checks
   getData() before accessing chart-specific properties (.tree,
   .mapDimension, .getGraph, etc.)

Fixes apache#9402, apache#16998
@echarts-bot
Copy link

echarts-bot bot commented Mar 10, 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.

@davesecops davesecops closed this Mar 11, 2026
@davesecops davesecops deleted the fix/null-guard-getDataParams branch March 11, 2026 00:26
@davesecops
Copy link
Author

Superseded by #21537 (branch renamed to follow project convention fix-<issue_id>). Same fix + added unit tests.

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