codeant-ai-for-open-source[bot] commented on code in PR #37218:
URL: https://github.com/apache/superset/pull/37218#discussion_r2722093807
##########
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx:
##########
@@ -17,7 +17,11 @@
* under the License.
*/
import { t } from '@apache-superset/core';
-import { validateInteger, validateNonEmpty } from '@superset-ui/core';
+import {
+ validateInteger,
+ validateNonEmpty,
+ withLabel,
Review Comment:
**Suggestion:** Incorrect import of the default-exported `withLabel`: the
utility is exported as a default export but the PR imports it as a named
export, which will make `withLabel` undefined and cause a TypeError when called
at runtime; change the import to use the default import form. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Histogram editor throws TypeError blocking configuration.
- ❌ 'Bins' field validation fails preventing correct input.
- ⚠️ Chart save/edit operations may be interrupted.
```
</details>
```suggestion
import withLabel, {
validateInteger,
validateNonEmpty,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load the Chart edit UI and open the Histogram chart editor (renders
control panel file
at
superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx).
The
file imports withLabel at the top (lines 20-24).
2. Interact with the Bins control in the UI (the control is configured at
controlPanel.tsx: the validators invocation is at line 73: `validators:
[withLabel(validateInteger, t('Bins'))],`).
3. When the Bins control validation runs, the code calls the withLabel
function referenced
in controlPanel.tsx:73.
4. The withLabel implementation in the codebase is a default export (see
packages/superset-ui-core/src/utils/withLabel.ts:33-41, `export default
function
withLabel(...) { ... }`). Because controlPanel.tsx imports withLabel as a
named import,
the imported identifier will be undefined; calling it produces a runtime
TypeError
("withLabel is not a function") in the browser console and breaks validation
UI.
```
</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/Histogram/controlPanel.tsx
**Line:** 20:23
**Comment:**
*Possible Bug: Incorrect import of the default-exported `withLabel`:
the utility is exported as a default export but the PR imports it as a named
export, which will make `withLabel` undefined and cause a TypeError when called
at runtime; change the import to use the default import form.
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>
##########
superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts:
##########
@@ -20,13 +20,13 @@
import { validateMaxValue } from '@superset-ui/core';
Review Comment:
**Suggestion:** The test imports `validateMaxValue` as a named import but
the validator implementation is a default export; this mismatch can lead to
`undefined` being imported and cause runtime errors in tests—use a default
import to match the implementation. [type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Unit tests fail with TypeError on import mismatch.
- ⚠️ CI jobs running package tests may be blocked.
```
</details>
```suggestion
import validateMaxValue from '@superset-ui-core';
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the test import at
superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
line
20 which does a named import: import { validateMaxValue } from
'@superset-ui-core';
2. Inspect the implementation in
superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts
lines 21-26
which declares a default export: export default function
validateMaxValue(...){ ... }.
3. Run the tests (e.g., yarn test packages/superset-ui-core
--selectTestsByPath). If the
package root does not re-export validateMaxValue as a named export, the
named import will
be undefined and the first call at test line 24 (validateMaxValue(10.1, 10))
will throw a
TypeError: validateMaxValue is not a function.
4. Fixing the import to default (import validateMaxValue from
'@superset-ui-core';) aligns
the test import with the implementation and prevents the runtime failure.
5. Note: if the package root index explicitly re-exports validateMaxValue as
a named
export, the existing import is valid; however there is concrete evidence
here (the
implementation file shows a default export) that this mismatch is likely
unless a
re-export exists.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
**Line:** 20:20
**Comment:**
*Type Error: The test imports `validateMaxValue` as a named import but
the validator implementation is a default export; this mismatch can lead to
`undefined` being imported and cause runtime errors in tests—use a default
import to match the implementation.
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]
