bito-code-review[bot] commented on PR #41111:
URL: https://github.com/apache/superset/pull/41111#issuecomment-4716244385
<!-- Bito Reply -->
The flagged issue is correct. In `Echart.tsx`, the `useEffect` hook re-binds
query listeners on every render where `queryEventHandlers` changes. Because
`chartRef.current?.off(name, handler)` is called with the *current* handler
instance, if the handler was recreated (e.g., due to dependency changes in
`useCallback`), the previous handler remains registered, leading to multiple
event triggers.
To resolve this, you should clear all query listeners for the specific event
name before re-binding, or ensure the handler reference is stable. A minimal
fix is to use `chartRef.current?.off(name)` without the handler argument to
remove all listeners for that event name before re-binding:
```tsx
(queryEventHandlers || []).forEach(({ name, query, handler }) => {
chartRef.current?.off(name);
chartRef.current?.on(name, query, handler);
});
```
This ensures that any previously registered handlers for that event name are
cleared, preventing duplicate triggers.
**superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx**
```
(queryEventHandlers || []).forEach(({ name, query, handler }) => {
chartRef.current?.off(name);
chartRef.current?.on(name, query, handler);
});
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
