You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/09/11 20:19:48 UTC

[PR] refactor(frontend): consolidate stray antd wrappers into superset-ui-core [superset]

rusackas opened a new pull request, #44177:
URL: https://github.com/apache/superset/pull/44177

   ### SUMMARY
   Follow-up to a frontend DRY audit. Original grep for direct `antd` imports outside `@superset-ui/core/components` found ~79 hits, but almost all of those turned out to be that wrapper package's own legitimate internals (it has to import `antd` to wrap it) plus a handful of theme-system test files -- not real violations. The actual, actionable set was much smaller:
   
   **Three components wrapped ad hoc at the app level instead of living in the core package.** `Splitter`, `Descriptions`, and `TimePicker`/`TimeRangePicker` each had a thin one-off wrapper under `src/components/X`, not because anyone was avoiding the convention, but because no wrapper existed yet for these three in `@superset-ui/core/components` (confirmed: no `Splitter`/`Descriptions`/`TimePicker` directory there before this PR). Moved all three into the core package alongside every other antd wrapper, following the same export pattern already used for ~150 other components in `components/index.ts`, and updated the six call sites. Component bodies are byte-identical -- this only changes where they live and how they're imported.
   
   **One real "should have used the existing wrapper" case.** `plugin-chart-ag-grid-table`'s `TimeComparisonVisibility.tsx` imported `Dropdown` straight from `antd`, under an `eslint-disable-next-line import/no-extraneous-dependencies` (since `antd` isn't a declared dependency of that plugin) -- even though a `Dropdown` wrapper already exists in `@superset-ui/core/components`, which *is* a real dependency of the plugin. Swapped the import and dropped the now-unnecessary suppression.
   
   ### TESTING INSTRUCTIONS
   - `tsc -b` clean across `superset-ui-core`, `plugin-chart-ag-grid-table`, and the full app project (only pre-existing errors on completely unrelated, untouched files remain, e.g. `SafeMarkdown.stories.tsx`).
   - `oxlint` clean on every changed file.
   - The three affected Jest suites pass: `AppLayout.test.tsx`, `UserInfo.test.tsx`, `SqlEditor.test.tsx` -- 27 tests, including the `AppLayout` mock now targeting the new `@superset-ui/core/components/Splitter` subpath instead of the old `src/components/Splitter` path (same pattern already used for the neighboring `Grid` mock in that file).
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] refactor(frontend): consolidate stray antd wrappers into superset-ui-core [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44177:
URL: https://github.com/apache/superset/pull/44177#issuecomment-5640164345

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] refactor(frontend): consolidate stray antd wrappers into superset-ui-core [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #44177:
URL: https://github.com/apache/superset/pull/44177


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] refactor(frontend): consolidate stray antd wrappers into superset-ui-core [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44177:
URL: https://github.com/apache/superset/pull/44177#issuecomment-5640319217

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44177?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 80.16%. Comparing base ([`36361d9`](https://app.codecov.io/gh/apache/superset/commit/36361d9d0462b34114d4be0a87ca78d7a576f387?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f506a9e`](https://app.codecov.io/gh/apache/superset/commit/f506a9e5d1379fa11ba53380944e42facb504ad7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #44177       +/-   ##
   ===========================================
   + Coverage   66.18%   80.16%   +13.98%     
   ===========================================
     Files        2925     2925               
     Lines      172563   172563               
     Branches    40058    40058               
   ===========================================
   + Hits       114212   138337    +24125     
   + Misses      55970    31629    -24341     
   - Partials     2381     2597      +216     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/44177/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/44177/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `75.66% <ø> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44177?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org