Only unbind events passed from the parent component#613
Merged
hustcc merged 1 commit intohustcc:masterfrom Nov 6, 2025
Merged
Conversation
`initEchartsInstance` attached a handler for the `"finish"` event. If `componentDidUpdate` is called between attaching the handler, and firing the handler and the `onEvents` prop changed, this will fire `"offEvents"` which will remove the handler for the `"finish"` event. This means the finish handler never fires, so the chart auto-resize is not attached, and the ready event handler never fires. Instead, only remove the events that are on the previous props, and leave the `"finish"` callback alone.
Contributor
Author
|
@hustcc would you mind taking a look? I'm happy to make any necessary changes, but as far as I can tell this is a pretty straight-forward bug fix. |
gggritso
added a commit
to getsentry/sentry
that referenced
this pull request
Nov 6, 2025
There's a bug in Explore charts, where the chart doesn't resize correctly when the browser size changes. This bug is caused by a wacky race condition in `echarts-for-react`, which was fixed in hustcc/echarts-for-react#613 by yours truly, and made its way into 3.0.5
Ahmed-Labs
pushed a commit
to getsentry/sentry
that referenced
this pull request
Nov 6, 2025
There's a bug in Explore charts, where the chart doesn't resize correctly when the browser size changes. This bug is caused by a wacky race condition in `echarts-for-react`, which was fixed in hustcc/echarts-for-react#613 by yours truly, and made its way into 3.0.5
Jesse-Box
pushed a commit
to getsentry/sentry
that referenced
this pull request
Nov 12, 2025
There's a bug in Explore charts, where the chart doesn't resize correctly when the browser size changes. This bug is caused by a wacky race condition in `echarts-for-react`, which was fixed in hustcc/echarts-for-react#613 by yours truly, and made its way into 3.0.5
andrewshie-sentry
pushed a commit
to getsentry/sentry
that referenced
this pull request
Nov 13, 2025
There's a bug in Explore charts, where the chart doesn't resize correctly when the browser size changes. This bug is caused by a wacky race condition in `echarts-for-react`, which was fixed in hustcc/echarts-for-react#613 by yours truly, and made its way into 3.0.5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is a rare race condition in
echarts-for-reactwhere the"finished"event might not fire if the component props were updated very soon after initialization. There is a repro case in this repo: https://github.com/gggritso/echarts-raceTo run the repro case, run
npm install, and thennpm run dev. When the page loads, you'll see that the chart does not automatically resize when the page changes.Screen.Recording.2025-11-04.at.1.50.48.PM.mov
Here are the execution steps:
EChartsReactCoremounts.this.initEchartsInstanceis called, and attaches a handler for the"finished"eventuseLayoutEffectcall.EChartsReactCoreis re-rendered, andcomponentDidUpdateis called. It sees that theonEventsprop changed, and callsoffEventswith all the eventsoffEventsiterates through the events, and unbinds all of them usinginstance.off.eventshas a"finished"handler, sooffEventsremoves all finish handlers including the one added byinitiEchartsInstance, since it doesn't specify which handler to remove"finished"callback never fires, so resize observer is never attachedThis PR makes a change where
offEventsonly removes the handlers that changed between the renders, rather than all handlers. This preserves the"finished"handler that was attached during mount.