codeant-ai-for-open-source[bot] commented on code in PR #37532:
URL: https://github.com/apache/superset/pull/37532#discussion_r2746571817
##########
superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx:
##########
@@ -148,6 +152,14 @@ export const stackControl: ControlSetItem = {
},
};
+export const stackControlWithoutStream: ControlSetItem = {
+ ...stackControl,
+ config: {
+ ...stackControl.config,
+ choices: StackControlOptionsWithoutStream,
Review Comment:
**Suggestion:** Shallow-spread of `stackControl` to create
`stackControlWithoutStream` can unintentionally carry over properties beyond
`config` (including functions or references), creating subtle shared-state or
mutation bugs; create a new, explicit ControlSetItem with only the expected
fields (name + config) so the new control is an independent object. [possible
bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries control panel UI inconsistencies.
- ⚠️ Chart editor form state collisions for stack field.
```
</details>
```suggestion
name: 'stack',
config: {
type: 'SelectControl',
label: t('Stacked Style'),
renderTrigger: true,
choices: StackControlOptionsWithoutStream,
default: null,
description: t('Stack series on top of each other'),
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the control definitions in
superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx and locate
the original
stack control defined at lines 143-153 (export const stackControl).
2. Observe the variant at lines 155-161 (export const
stackControlWithoutStream) that is
created by shallow-spreading stackControl and stackControl.config. This is
the exact code
shown at 155..161 in this file.
3. In a consumer (the Timeseries control panels), import both
showValueSection (which
contains stackControl at lines 196..201) and showValueSectionWithoutStream
(which contains
stackControlWithoutStream at lines 208..213) into the same panel definition
(a realistic
change when composing a control panel for a mixed chart).
4. Render the chart editor with both sections included. Because both control
objects
reference the same nested objects via shallow spread, mutating code in the
control
renderer (e.g., adding listeners, augmenting config) can modify the shared
nested config
in-place, producing unexpected UI state or cross-control side-effects. If
you want to
verify locally, add a small mutation hook in the editor that modifies a
control.config
property at runtime and observe both controls reflect the change
(reproducible by
instrumenting the editor that consumes these exports).
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx
**Line:** 156:159
**Comment:**
*Possible Bug: Shallow-spread of `stackControl` to create
`stackControlWithoutStream` can unintentionally carry over properties beyond
`config` (including functions or references), creating subtle shared-state or
mutation bugs; create a new, explicit ControlSetItem with only the expected
fields (name + config) so the new control is an independent object.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
