bito-code-review[bot] commented on code in PR #40919:
URL: https://github.com/apache/superset/pull/40919#discussion_r4167617475
##########
superset-frontend/src/utils/common.ts:
##########
@@ -24,15 +24,16 @@ import {
JsonObject,
} from '@superset-ui/core';
-// ATTENTION: If you change any constants, make sure to also change
constants.py
+import {
+ EMPTY_STRING,
+ NULL_STRING,
+ TRUE_STRING,
+ FALSE_STRING,
+} from '../../plugins/plugin-chart-echarts/src/constants';
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Core depends on plugin</b></div>
<div id="fix">
Core `src/utils/common.ts` now imports
`EMPTY_STRING`/`NULL_STRING`/`TRUE_STRING`/`FALSE_STRING` from
`plugins/plugin-chart-echarts/src/constants`, inverting the dependency
direction: core utilities now depend on a specific chart plugin package. This
couples the core app to a plugin and breaks the plugin-depends-on-core
layering. Consider a shared constants module.
</div>
</div>
<small><i>Code Review Run #1ce886</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/components/FilterableTable/useCellContentParser.ts:
##########
@@ -17,10 +17,11 @@
* under the License.
*/
import { useCallback, useMemo } from 'react';
+import { t } from '@apache-superset/core/translation';
export type CellDataType = string | number | null;
-export const NULL_STRING = 'NULL';
+export const NULL_STRING = () => t('NULL');
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>duplicate NULL_STRING constant</b></div>
<div id="fix">
Line 24 introduces a second `NULL_STRING` function (`() => t('NULL')`) while
`plugin-chart-echarts/src/constants.ts:31` already exports `NULL_STRING = () =>
t('<NULL>')` re-exported via `src/utils/common`. Two same-named constants with
different translated strings ('NULL' vs '<NULL>') invite importing the wrong
one and divergent null displays. Consider reusing the shared symbol or renaming
this one.
</div>
</div>
<small><i>Code Review Run #1ce886</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]
