chore(frontend): enable additional oxlint rules for better code hygiene#38145
Merged
chore(frontend): enable additional oxlint rules for better code hygiene#38145
Conversation
|
Related Documentation Checked 0 published document(s) in 2 knowledge base(s). No updates required. How did I do? Any feedback? |
Contributor
Code Review Agent Run #5f36b8Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
Outdated
Show resolved
Hide resolved
Contributor
Code Review Agent Run #aa31d5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Enable 17 new oxlint rules that enforce best practices:
**Zero-violation rules (enabled as errors):**
- eslint/no-useless-constructor - Remove empty constructors
- eslint/no-else-return - Unnecessary else after return
- eslint/no-array-constructor - Use [] instead of new Array()
- eslint/no-new-wrappers - Don't use new String/Number/Boolean()
- eslint/no-regex-spaces - Use {n} instead of multiple spaces
- eslint/no-object-constructor - Use {} instead of new Object()
- unicorn/no-length-as-slice-end - Don't pass .length to .slice() end
- unicorn/no-useless-spread - Remove unnecessary spreads
- unicorn/no-thenable - Don't export .then on non-Promises
- unicorn/escape-case - Consistent escape character casing
**Small fix rules (auto-fixed violations):**
- unicorn/prefer-array-flat-map - Use .flatMap() instead of .map().flat()
- unicorn/prefer-array-some - Use .some() instead of .findIndex() !== -1
- unicorn/throw-new-error - Use throw new Error() not throw Error()
- unicorn/prefer-negative-index - Use .at(-1) for negative indices
- unicorn/prefer-math-trunc - Use Math.trunc() instead of | 0
Also adds the oxc plugin to enable additional rule coverage (193 -> 205 rules).
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The column key was using t('Main') (translated) while the data records
used a hard-coded "Main " prefix. In non-English locales this caused
empty cells because the table looked for translated keys that didn't
exist in the data.
Keep label translated for UI display, but use stable "Main " for the
key to match processComparisonDataRecords and processComparisonTotals.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…alStorage migration Removes the reassignment that overwrote the backend-sourced lastUpdatedActiveTab with the last entry from merged tabHistory. This was breaking EditorAutoSync and persistSqlLabStateEnhancer which rely on detecting when the active tab differs from lastUpdatedActiveTab to push state to the backend. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ssigned Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rusackas
force-pushed
the
chore/enable-additional-oxlint-rules
branch
from
5f2c356 to
56e8e63
Compare
| originalLabel, | ||
| label: t('Main'), | ||
| key: `Main ${col.key}`, | ||
| config: getComparisonColConfig('Main', col.key, columnConfig), |
Contributor
There was a problem hiding this comment.
Suggestion: The comparison column config lookup for the "Main" column uses the hard-coded string 'Main' instead of the localized label used when generating column names in the control panel (t('Main')), so in non-English locales any custom column configuration (formatting, currency, etc.) for these "Main" comparison columns will never be matched or applied. [logic error]
Severity Level: Major ⚠️
- ⚠️ Main comparison columns ignore custom formatting in localized UIs.
- ⚠️ Main comparison columns cannot be hidden via column_config settings.
Suggested change
| config: getComparisonColConfig('Main', col.key, columnConfig), | |
| config: getComparisonColConfig(t('Main'), col.key, columnConfig), |
Steps of Reproduction ✅
1. In Explore, create or edit a Table chart using the legacy table plugin
(`plugins/plugin-chart-table`) with time comparison enabled so `comparison_type ===
ComparisonType.Values` and `time_compare` is non-empty (checked in
`transformProps.ts:535-538`).
2. With the UI language set to a non-English locale, open the chart's Customize columns
control (`column_config` control defined in
`plugins/plugin-chart-table/src/controlPanel.tsx:556-649`).
3. For a metric like `metric_1`, configure formatting/visibility on its "Main" comparison
column; the control panel generates column_config keys using the localized label, e.g.
`${t('Main')} metric_1` in `generateComparisonColumns` at `controlPanel.tsx:158-166`, and
uses those names when building `column_config`.
4. Run the chart; `transformProps` (`transformProps.ts:490-795`) calls
`processComparisonColumns` (`transformProps.ts:382-467`), which creates a Main column with
`label: t('Main')` but looks up config via `getComparisonColConfig('Main', col.key,
columnConfig)` at `transformProps.ts:406`; `getComparisonColConfig`
(`transformProps.ts:340-347`) builds the key `${label} ${parentColKey}` which becomes
`'Main metric_1'` instead of the localized `${t('Main')} metric_1`, so it returns `{}` and
the custom column_config for the Main comparison column is never applied in non-English
locales.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
**Line:** 406:406
**Comment:**
*Logic Error: The comparison column config lookup for the "Main" column uses the hard-coded string 'Main' instead of the localized label used when generating column names in the control panel (`t('Main')`), so in non-English locales any custom column configuration (formatting, currency, etc.) for these "Main" comparison columns will never be matched or applied.
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.
Contributor
Code Review Agent Run #53d3d3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Code Review Agent Run #2a0e3eActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
hainenber
approved these changes
Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR enables 17 new oxlint rules that enforce JavaScript/TypeScript best practices. These rules were either already satisfied (zero violations) or had small numbers of auto-fixable violations.
Zero-violation rules (enabled as errors)
eslint/no-useless-constructoreslint/no-else-returneslint/no-array-constructor[]instead ofnew Array()eslint/no-new-wrappersnew String/Number/Boolean()eslint/no-regex-spaces{n}instead of multiple spaces in regexeslint/no-object-constructor{}instead ofnew Object()unicorn/no-length-as-slice-end.lengthto.slice()endunicorn/no-useless-spreadunicorn/no-thenable.thenon non-Promisesunicorn/escape-caseAuto-fixed rules (small number of violations)
unicorn/prefer-array-flat-map.flatMap()instead of.map().flat()unicorn/prefer-array-some.some()instead of.findIndex() !== -1unicorn/throw-new-errorthrow new Error()notthrow Error()unicorn/prefer-negative-index.at(-1)for negative indicesunicorn/prefer-math-truncMath.trunc()instead of `Also adds the
oxcplugin to enable additional rule coverage (179 → 205 rules).Test plan
npm run lintpasses with 0 errors🤖 Generated with Claude Code