codeant-ai-for-open-source[bot] commented on code in PR #37107:
URL: https://github.com/apache/superset/pull/37107#discussion_r2692555055
##########
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx:
##########
@@ -115,7 +115,7 @@ export default class CRUDCollection extends PureComponent<
}
}
- onCellChange(id: number, col: string, val: boolean) {
+ onCellChange(id: number, col: string, val: unknown) {
Review Comment:
**Suggestion:** The `onCellChange` method's `id` parameter is typed as
`number`, but collection item IDs are defined as `string | number` and are
passed directly from `record.id`, so this overly narrow type will cause
TypeScript errors at the call site and incorrectly constrains valid string IDs.
[type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Type-check fails blocking build and CI.
- ❌ Table editing callbacks broken during compile.
- ⚠️ Developer workflow interrupted by type errors.
```
</details>
```suggestion
onCellChange(id: string | number, col: string, val: unknown) {
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the file
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx
and
locate the onCellChange declaration at line 118 which is typed as
onCellChange(id: number,
col: string, val: unknown) { (file lines verified via Read).
2. Locate the caller in the same file: renderCell binds the handler at line
307 with
this.onCellChange.bind(this, record.id, col) where record.id comes from the
CollectionItem
type declared at line 55 as type CollectionItem = { id: string | number; ...
} (verified
via Read lines 55 and 304-308). This passes a string | number into a
function expecting
number.
3. Run TypeScript type checking (npm run type) or tsc; the call site
this.onCellChange.bind(this, record.id, col) will produce a type error
because record.id
has type string | number but onCellChange expects number (type mismatch
discovered by
tracing types in-file).
4. Additional usages: deleteItem is declared with id: string | number at
line 225 and
deleteItem(record.id) is invoked at line 383 — this shows the codebase
consistently treats
ids as string | number, making the numeric-only onCellChange signature
inconsistent
(verified via Read at lines 225 and 383). Because callers pass string IDs
(nanoid-generated), the narrow type causes real compilation failures.
5. Expected reproduction outcome: TypeScript reports an argument type
mismatch at the call
site (renderCell line ~307) or when binding the function, blocking type
checking and CI
type tasks.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/components/CollectionTable/index.tsx
**Line:** 118:118
**Comment:**
*Type Error: The `onCellChange` method's `id` parameter is typed as
`number`, but collection item IDs are defined as `string | number` and are
passed directly from `record.id`, so this overly narrow type will cause
TypeScript errors at the call site and incorrectly constrains valid string IDs.
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]
