bito-code-review[bot] commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3538098754
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts:
##########
@@ -125,7 +125,7 @@ export function useFilterOperations({
);
const handleRearrangeFilters = useCallback(
- (dragIndex: number, targetIndex: number, id: string) => {
+ (dragIndex: number, targetIndex: number, _id: string) => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unused parameter in public API</b></div>
<div id="fix">
The `id` parameter is not used in the function body. Prefixing with `_id`
suppresses the warning but leaves dead parameter in the public API. Remove `id`
from both the interface and implementation for a clean API contract.
</div>
</div>
<small><i>Code Review Run #b83f72</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/ExploreAlert.tsx:
##########
@@ -51,7 +51,7 @@ export const ExploreAlert = forwardRef(
type = 'info',
className = '',
}: ControlPanelAlertProps,
- ref: RefObject<HTMLDivElement>,
+ _ref: RefObject<HTMLDivElement>,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unused ref in forwardRef component</b></div>
<div id="fix">
The `ref` parameter (renamed to `_ref`) is never used in the component body
(lines 55-77). Keeping `forwardRef` while ignoring the forwarded ref is
misleading — it suggests the component exposes a DOM ref that callers can use,
but the ref is silently dropped. Either remove `forwardRef` entirely if ref
forwarding isn't needed, or forward the ref to an appropriate DOM element if
future usage is intended.
</div>
</div>
<small><i>Code Review Run #b83f72</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx:
##########
@@ -92,7 +92,7 @@ const MatrixNoDataComponent = () => {
* Individual grid cell component - memoized to prevent unnecessary re-renders
*/
const MatrixifyGridCell = memo(
- ({ cell, rowHeight, datasource, hooks }: MatrixifyGridCellProps) => {
+ ({ cell, hooks }: MatrixifyGridCellProps) => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing prop causes runtime crash</b></div>
<div id="fix">
The component destructures `({ cell, hooks })` but the custom memo
comparator still references `prevProps.rowHeight` and `nextProps.rowHeight` on
lines 178-179, which will cause a runtime `TypeError` when React's memo
attempts to compare props. Additionally, the interface at line 80 still
declares `rowHeight: number` as required, creating a TypeScript/JSX overload
mismatch. Remove `rowHeight` from the interface, the destructuring, and the
comparator check.
</div>
</div>
<small><i>Code Review Run #b83f72</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx:
##########
@@ -34,7 +34,7 @@ export const Label = forwardRef<HTMLSpanElement,
LabelProps>((props, ref) => {
onClick,
children,
icon,
- id,
+ id: _id,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Dead code drops id prop</b></div>
<div id="fix">
The `id` prop is destructured and renamed to `_id`, but `_id` is never used.
This silently drops the `id` prop that callers pass, breaking the component's
contract with consumers who rely on `id` for testing, accessibility, or CSS
targeting. Revert to `id,` so it passes through `{...rest}` to `<Tag>`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx
(lines 34-40) ---
34: onClick,
35: children,
36: icon,
37: - id: _id,
37: + id,
38: ...rest
39: } = props;
```
</div>
</details>
</div>
<small><i>Code Review Run #85ca94</i></small>
</div><div>
<div id="suggestion">
<div id="issue"><b>CWE-20: Broken id prop forwarding</b></div>
<div id="fix">
The `id` prop is destructured but never passed to the `Tag` component,
breaking prop forwarding. Users calling `<Label id="my-id" />` will find the id
attribute missing from the rendered element. Since `id` is defined in
`LabelProps` via `HTMLAttributes<HTMLSpanElement>` and `Tag` accepts id, simply
exclude it from destructuring to allow it to flow through `...rest`. (See also:
[CWE-20](https://cwe.mitre.org/data/definitions/20.html))
</div>
</div>
<small><i>Code Review Run #b83f72</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
