bito-code-review[bot] commented on code in PR #41851:
URL: https://github.com/apache/superset/pull/41851#discussion_r3545105331
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts:
##########
@@ -445,6 +449,7 @@ const processColumns = memoizeOne(function processColumns(
isPercentMetric,
formatter,
config,
+ type: columnsByName.get(key)?.type,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing test for new type field</b></div>
<div id="fix">
The new `type` field is correctly populated from `props.datasource.columns`
via `columnsByName.get(key)?.type`, matching the interface contract (`type?:
string` on `DataColumnMeta`) and the pattern in `plugin-chart-table`. However,
no existing test verifies this behavior. Add at least one test asserting that
`columnMeta.type` reflects the datasource column type to prevent future
regressions.
</div>
</div>
<small><i>Code Review Run #445a94</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/plugins/plugin-chart-table/src/utils/isEqualColumns.ts:
##########
@@ -44,6 +44,7 @@ export default function isEqualColumns(
JSON.stringify(a.formData.extraFormData || null) ===
JSON.stringify(b.formData.extraFormData || null) &&
JSON.stringify(a.rawFormData.column_config || null) ===
- JSON.stringify(b.rawFormData.column_config || null)
+ JSON.stringify(b.rawFormData.column_config || null) &&
+ a.datasource.columns === b.datasource.columns
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Shallow array comparison breaks memoization</b></div>
<div id="fix">
The `===` operator compares arrays by reference, not value. When
`datasource.columns` is an array of objects (as used in
`transformProps.ts:236`), two logically identical column arrays from different
object instances will fail comparison, causing `processColumns` to re-execute
instead of using the memoized result. Use `isEqualArray` which already exists
in this file at line 19.
</div>
</div>
<small><i>Code Review Run #445a94</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]
