amad3v commented on code in PR #21201:
URL: https://github.com/apache/echarts/pull/21201#discussion_r4233735446
##########
src/model/Global.ts:
##########
@@ -1039,7 +1035,9 @@ function mergeTheme(option: ECUnitOption, theme:
ThemeOption): void {
if (typeof themeItem === 'object') {
option[name] = !option[name]
? clone(themeItem)
- : merge(option[name], themeItem, false);
+ : preserveUserOptions
+ ? merge(themeItem, option[name], false) // User
options have higher priority
Review Comment:
> Currently, I think the quick fix proposed should not be applied, since it
brings more bugs that cannot to be fixed.
>
> > So it seems that if we need to solve this problem now
>
> Could you explain the original scenario where you need to use setTheme
while keeping data? I’d like to determine whether this usage is appropriate,
and whether a major refactor is worth it.
Real-world scenarios, since that is the open question:
1. **Dark-mode toggle on incrementally updated charts.** Dashboards that
poll or stream data apply each update with `setOption` in merge mode (only the
changed series or data). When the user switches the app to dark mode,
`setTheme` brings back the option from the first `setOption`, so the chart
shows stale data until the next update arrives.
2. **Framework wrappers.** Wrappers apply reactive option changes in merge
mode and expose `theme` as a prop. A theme prop change therefore reverts the
chart. Two that hit this:
-
[jmix-framework/jmix#5699](https://github.com/jmix-framework/jmix/issues/5699)
stopped using `setTheme` and recreates the instance on every theme change.
- [@amad3v/solid-echarts](https://codeberg.org/amad3v/solid-echarts)
calls `setOption` with the current option again right after every `setTheme`.
3. **Why `notMerge: true` everywhere does not solve it:** every update must
then carry the complete option, and the whole model is rebuilt each time, which
loses the point of merge mode for incremental updates.
A small correction to the workaround above: in merge mode it is the
**first** option that survives `setTheme`, not the last.
[`_optionBackup`](https://github.com/apache/echarts/blob/6.1.0/src/model/OptionManager.ts#L148-L150)
is only assigned on the first `setOption`, and
[`mountOption(true)`](https://github.com/apache/echarts/blob/6.1.0/src/model/OptionManager.ts#L153-L168)
restores it:
```js
chart.setOption(option([1, 2, 3]));
chart.setOption(option([4, 5, 6])); // merge mode
chart.setTheme('dark');
chart.getOption().series[0].data; // [1, 2, 3]
```
--
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]
