fix(core): fix potential NPE when reading default init options from global object in dev environment #19217
Conversation
|
Thanks for your contribution! |
|
Very good, I have also encountered this issue and hope to merge |
src/core/echarts.ts
Outdated
| @@ -418,11 +418,11 @@ class ECharts extends Eventful<ECEventDefinition> { | |||
| env.hasGlobalWindow ? window : global | |||
| ) as any; | |||
|
|
|||
| defaultRenderer = root.__ECHARTS__DEFAULT__RENDERER__ || defaultRenderer; | |||
| defaultRenderer = root?.__ECHARTS__DEFAULT__RENDERER__ ?? defaultRenderer; | |||
There was a problem hiding this comment.
As the build target of ECharts is currently still ES3, to reduce the transpiled code size as possible, instead of ?. and ??, just fallback root to {} when it is not defined or simply surround the block (line 421- line 428) with if (root) {}.
There was a problem hiding this comment.
You're right. I overlooked that. I'll revise it.
src/core/echarts.ts
Outdated
| if (__DEV__) { | ||
| let devUseDirtyRect = null; |
src/core/echarts.ts
Outdated
| defaultUseDirtyRect = devUseDirtyRect == null | ||
| ? defaultUseDirtyRect | ||
| : devUseDirtyRect; |
src/core/echarts.ts
Outdated
| defaultRenderer = root.__ECHARTS__DEFAULT__RENDERER__ || defaultRenderer; | ||
| if (root) { | ||
| defaultRenderer = root.__ECHARTS__DEFAULT__RENDERER__ || defaultRenderer; | ||
| devUseDirtyRect = root.__ECHARTS__DEFAULT__USE_DIRTY_RECT__; |
There was a problem hiding this comment.
Change to:
defaultUseDirtyRect = retrieve2(root.__ECHARTS__DEFAULT__USE_DIRTY_RECT__, defaultUseDirtyRect);
|
I don’t know if this modification is correct? |
|
Thank you for your careful guidance |
plainheart
changed the title
|
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
Brief Information
This pull request is in the type of:
What does this PR do?
The value of repairing the development environment
__ECHARTS__DEFAULT__RENDERER__is undefinedDetails
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