Skip to content

feat(ui): Wave 0 foundations for AntD → ui-core-components migration - #30560

Open
chirag-madlani wants to merge 18 commits into
mainfrom
antd-migration/wave-0-foundations
Open

feat(ui): Wave 0 foundations for AntD → ui-core-components migration#30560
chirag-madlani wants to merge 18 commits into
mainfrom
antd-migration/wave-0-foundations

Conversation

@chirag-madlani

@chirag-madlani chirag-madlani commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Foundations for the AntD → @openmetadata/ui-core-components migration program (Wave 0). No product code changes — tooling, docs, test harness, and CI only.

  • docs/antd-migration/ — sweep playbook README + mapping-guide template (the review contract for every future component sweep)
  • tooling/antd-codemods/ — jscodeshift scaffold with a generic move-named-imports transform (6 inline tests: partial move, full-move removal, alias preservation, merge into existing import, no-op, license-header retention)
  • tooling/antd-migration/ledger.mjs — sweep-ledger generator counting antd imports per component per repo plus .ant-* selector occurrences (5 node:test cases; skips vendored/generated dirs; QueryBuilder files excluded as owned by feat(query-builder): migrate QueryBuilderWidgetV1 from antd to core-components #29849)
  • Visual-regression harness — new visual-regression Playwright project (separate from e2e; chromium project ignores it), determinism helper (fixed clock before navigation, animation freeze, shared screenshot opts), and 16 committed baselines: 13 static pages (incl. collapsed-sidebar variant), table entity details (API-provisioned fixture, name masks), RJSF connector config form, delete-confirmation modal
  • .github/workflows/playwright-visual.yml — PR gate for openmetadata-ui/** running the visual suite inside mcr.microsoft.com/playwright:v1.57.0-jammy (same image the baselines were generated in, for font-rendering parity), with disk-space reclaim and failure-artifact upload

Verification

  • Baselines were generated in the Playwright docker image (Linux rendering) against a locally seeded server, and the full suite passed two consecutive verification runs (18/18 each) with zero diffs.
  • tooling/antd-codemods: yarn test 6/6 · tooling/antd-migration: node --test 5/5 · eslint/prettier clean on all new spec files · actionlint clean on the workflow.
  • knip seed list (docs/antd-migration/dead-code-seed.json, 8 files / 451 issues): 3 entries grep-spot-checked, 0 false positives (e.g. DataAssetsHeaderExtraInfo has zero importers). Seed is advisory — deletions must be re-proven in the deleting PR.

Notes for reviewers

  • The visual workflow's first live run is the real end-to-end test of the docker/host-networking path in CI; if runner font rendering differs from the pinned image expectations, a one-time --update-snapshots pass regenerates baselines.
  • A sibling PR in the Collate repo (cross-repo ledger + collate-ui visual project) references docs added here, so this PR should merge first.

🤖 Generated with Claude Code

Fixes #30564 (Wave 0 of epic #30570)

Greptile Summary

The PR establishes the first migration-wave infrastructure:

  • Adds AntD migration documentation, a usage ledger, and a named-import codemod.
  • Adds a deterministic Playwright visual-regression project with committed baselines.
  • Adds a dedicated visual-regression workflow and delegates those specs away from the main Playwright planner.

Confidence Score: 4/5

The PR is not yet safe to merge because the codemod still converts inline type-only specifiers in mixed imports into runtime value imports.

A mixed declaration such as import { Button, type TabsProps } from 'antd' is classified by declaration kind as a value import, while the move loop ignores the selected specifier's own type kind; generated migrations can therefore fail TypeScript compilation or request a nonexistent runtime export.

Files Needing Attention: tooling/antd-codemods/transforms/move-named-imports.js; tooling/antd-codemods/tests/move-named-imports.test.js

Important Files Changed

Filename Overview
tooling/antd-codemods/transforms/move-named-imports.js Adds declaration-kind-aware import migration, but the outstanding mixed-import inline-type defect remains.
tooling/antd-codemods/tests/move-named-imports.test.js Covers ordinary and declaration-level type-only imports, aliases, merging, no-ops, and comment retention.
tooling/antd-migration/ledger.mjs Adds the AntD usage and selector ledger generator.
openmetadata-ui/src/main/resources/ui/playwright.config.ts Registers the isolated visual-regression Playwright project.
openmetadata-ui/src/main/resources/ui/playwright/utils/visualRegression.ts Centralizes deterministic clock, animation, masking, and screenshot behavior.
.github/workflows/playwright-visual.yml Adds the pinned-container visual-regression CI workflow and failure artifact upload.
.github/playwright/impact-map.json Delegates visual-regression specs away from the primary Playwright workflow.

Reviews (5): Last reviewed commit: "test(ui): widen landing-page diff tolera..." | Re-trigger Greptile

chirag-madlani and others added 9 commits July 27, 2026 19:01
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sform

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 09:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Jul 28, 2026
Comment thread tooling/antd-migration/ledger.mjs
Comment thread tooling/antd-codemods/transforms/move-named-imports.js Outdated
Comment thread tooling/antd-codemods/transforms/move-named-imports.js
Comment thread .github/workflows/playwright-visual.yml
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit df5fb41c693a1cb5e15aa08f4aadefb8c4411c9b in Playwright run 30391669318, attempt 1.

✅ 537 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 5 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 48m 5s

⏱️ Max setup 2m 54s · max shard execution 14m 21s · max shard-job elapsed before upload 17m 23s · reporting 4s

🌐 203.01 requests/attempt · 2.86 app boots/UI scenario · 18.58% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 18.58% (convergence target: at most 15%).
  • Browser traffic was 203.01 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.86 per UI scenario (1597 boots / 558 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 97 0 0 0 0 0
✅ Shard chromium-02 86 0 0 0 0 0
✅ Shard chromium-03 111 0 0 0 0 0
✅ Shard chromium-04 105 0 0 3 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 23 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 27 0 0 2 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@github-actions

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.67% (76800/116936) 49.54% (46114/93066) 50.76% (13892/27365)

chirag-madlani and others added 4 commits July 28, 2026 15:11
playwright/e2e/VisualRegression/*.spec.ts is testIgnore'd by the
chromium project (it runs under its own playwright-visual.yml
workflow), but select_playwright_tests.py still emitted a selector
for changed files under that directory. build_playwright_shards.py
then rejected the plan because the selector matched no runnable
chromium test, crashing "plan-playwright" for any PR touching the
visual suite.

Add playwright/e2e/VisualRegression/** to impact-map.json's
delegatedSpecs, the existing mechanism used for other
dedicated-workflow suites (Auth, nightly, Http2, RDF specs), so
changed visual-regression specs are routed to
delegatedChangedSpecs instead of selectors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reformat entityDetails.spec.ts to satisfy Prettier's line-length rule
(the UI Checkstyle job runs organize-imports-cli + eslint --fix +
prettier --write on changed playwright files and fails the PR if that
leaves a diff). Ran the same organize-imports/eslint/prettier chain
scoped to the four new visual-regression files; only this file needed
a change, and eslint + prettier --check are now clean on all of them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mports

parseAntdImports' named-import regex required `{` immediately after
import/type, so `import Button, { Divider } from 'antd';` contributed
nothing to the sweep ledger — silently undercounting any file that
mixes a default and named antd import on one line. Allow an optional
default binding before the braces.

Verified the change is a no-op on the current LEDGER.md: running
ledger.mjs against openmetadata-ui/src/main/resources/ui before and
after the regex change produces byte-identical output (0 files in
this repo currently use the combined default+named import shape).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ting import

When every specifier moves out of an antd import AND a target import
already exists, move-named-imports.js called `j(path).remove()` on the
antd import node, discarding its comments outright. If that import was
the file's first statement, its license header vanished from the
output.

Re-attach any leading comments to the node that becomes the program's
new first statement after removal (prepending them ahead of whatever
comments that node already carries).

Added a regression test with a license header above a first-statement
antd import merging into an already-imported target; it fails against
the old code (header dropped) and passes with the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 09:53
Comment on lines +13 to +15
function importKindOf(node) {
return node.importKind === 'type' ? 'type' : 'value';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Inline type imports become values

When a selected symbol uses mixed-import syntax such as import { Button, type TabsProps } from 'antd', importKindOf classifies the entire declaration as a value import and the value pass moves TabsProps accordingly. The destination then requests a runtime value for a type-only export, causing TypeScript compilation or module-loading failures.

Context Used: CLAUDE.md (source)

…riance

The landing dashboard's async widgets and version toast settle slightly
differently per CI run, hovering around the default 1% gate (1.06% on a
run following two green ones, with the internal 27%->2% retry progression
showing late-loading widget content). 3% absorbs the cross-run variance
without changing the rendered expectation, so the existing baseline stays
valid. Proper masking of the volatile widgets happens when the landing
page is reworked in its migration sweep.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 19:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 resolved / 2 findings

Establishes Wave 0 foundations for the AntD to ui-core-components migration with documentation, a named-import codemod, import ledger, and a Playwright visual-regression test harness. No issues found.

✅ 2 resolved
Edge Case: Ledger undercounts combined default+named antd imports

📄 tooling/antd-migration/ledger.mjs:20-34
In parseAntdImports, the named-import regex requires { immediately after import/type, so combined forms like import Button, { Divider } from 'antd'; do not match and their named specifiers (Divider) are never counted. Since this ledger drives migration completeness (a row is 'done' at zero), undercounting can hide remaining antd usage and mark a component prematurely complete. Consider matching an optional default binding before the braces, e.g. /import\s+(?:type\s+)?(?:[A-Za-z0-9_$]+\s*,\s*)?\{([^}]+)\}\s+from\s+['"]antd['"]/gs.

Edge Case: Codemod drops license header on full-move into existing import

📄 tooling/antd-codemods/transforms/move-named-imports.js:35-47
In the full-move branch (all specifiers listed), comments are only preserved on the no-existing-target path via decl.comments = path.node.comments; when a target import already exists the code calls j(path).remove(), which drops any leading comments attached to the antd import — including the license header when the antd import is the file's first statement. The test suite only covers header retention for the no-target replaceWith path. Consider re-attaching path.node.comments to the surviving first import (or the target) before removing.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wave 0: Foundations — visual-regression harness, codemods scaffold, sweep ledger

2 participants