fix(tooltip): fix potential NPE when setting option with notMerge strategy
#20435
Conversation
|
Thanks for your contribution! |
|
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-20435@50b715b |
| @@ -461,15 +461,15 @@ class TooltipHTMLContent { | |||
|
|
|||
| getSize() { | |||
| const el = this.el; | |||
| return [el.offsetWidth, el.offsetHeight]; | |||
| return [el?.offsetWidth, el?.offsetHeight]; | |||
There was a problem hiding this comment.
We try to avoid ?. in ECharts. Write explictly the expected value when el is not defined. Should it be something like el ? [el.offsetWidth, el.offsetHeight] : [0, 0] or what?
There was a problem hiding this comment.
Another reason is that ECharts will be eventually transpiled to ES3, and the optional chaining operator ?. will be polyfilled to:
return [el === null || el === void 0 ? void 0 : el.offsetWidth, el === null || el === void 0 ? void 0 : el.offsetHeight];It causes an unexpected increment in code size, which ECharts tries to avoid until we target ECharts to ES2020.
|
We sincerely appreciate your valuable contributions! We are currently running a special promotion for users who submit a Pull Request between September 25, 2024, and December 31, 2024. Eligible participants will receive a two-year enterprise membership to Baidu Comate (Please note that in order to register Comate, you need to have a phone number in China due to local regulations), valued at 2000 RMB (approximately 285 USD). To learn more about this product, please visit https://comate.baidu.com/. If you are interested in this offer, kindly send an email with the link to your Pull Request along with your Comate username and email address to ovilia@apache.org to claim your membership. Thank you once again for your support! |
| } | ||
|
|
||
| moveTo(zrX: number, zrY: number) { | ||
| const styleCoord = this._styleCoord; | ||
| makeStyleCoord(styleCoord, this._zr, this._container, zrX, zrY); | ||
|
|
||
| if (styleCoord[0] != null && styleCoord[1] != null) { | ||
| const style = this.el.style; | ||
| const style: any = this.el ? (this.el.style || {}) : {}; |
There was a problem hiding this comment.
Falling el.style back to {} makes no sense. It just circumvents those errors and causes unnecessary invoking. I would suggest:
moveTo(zrX: number, zrY: number) {
if (!this.el) {
return;
}
// ...
}
plainheart
changed the title
notMerge strategy
|
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
offsetWidthandoffsetHeightBrief Information
This pull request is in the type of:
What does this PR do?
Fixed issues
Details
Before: What was the problem?
After: How does it behave after the fixing?
Document Info
One of the following should be checked.
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information