Skip to content

perf(ui-bundle): split reactflow / recharts / rjsf / codemirror into vendor chunks#28016

Open
harshach wants to merge 1 commit into
mainfrom
harshach/perceived-latency-p3-2
Open

perf(ui-bundle): split reactflow / recharts / rjsf / codemirror into vendor chunks#28016
harshach wants to merge 1 commit into
mainfrom
harshach/perceived-latency-p3-2

Conversation

@harshach
Copy link
Copy Markdown
Collaborator

Describe your changes:

P3.2 of the perceived-latency plan tracked in .context/perceived-latency-design.md — bundle-split per tab.

Finding: the existing repo already wraps heavy tab components (Lineage, DataObservability, SampleData, TableQueries, Contract, KnowledgeGraph) in React.lazy() + Suspense via withSuspenseFallback. So the architectural lazy boundary was in place. But Rollup's automatic chunking heuristic was ignoring it: it lumped reactflow, recharts, rjsf, react-grid-layout, and codemirror into a single 8.87 MB grab-bag chunk (named after the first lazy entry to pull them in, AsyncDeleteProvider). Two effects:

  • On any tab change, the browser had to re-download or re-parse a chunk full of code unrelated to the tab the user clicked.
  • No browser-level parallel fetch — one giant chunk instead of several smaller cacheable ones.

Fix: add explicit manualChunks rules to Vite's rollup config so each heavy library gets its own named vendor chunk.

Before / after chunk sizes

Chunk Before After
AsyncDeleteProvider grab-bag 8.87 MB 7.71 MB (-13%)
vendor-reactflow (in grab-bag) 94 KB raw / 27 KB brotli
vendor-recharts (in grab-bag) 492 KB raw / 106 KB brotli
vendor-rjsf (in grab-bag) 267 KB raw / 77 KB brotli
vendor-codemirror (in grab-bag) 277 KB raw / 80 KB brotli

Each vendor chunk is independently cacheable across deploys (a recharts patch upgrade doesn't bust the rjsf cache) and fetched in parallel.

Subtle gotcha: reactflow is a meta-package that re-exports from @reactflow/* sub-packages (core, background, controls, minimap, node-resizer, node-toolbar). The heavy code lives in @reactflow/core, so the rule has to match both prefixes. Matching only reactflow produced no chunk at all on the first attempt.

Also fixed: three eager-path utils were value-importing types from reactflow (Node, Edge). Even though those symbols are pure types in reactflow's API, value imports drag the runtime into whatever chunk imports the util. EntityUtils.tsx is on the universal page-load path, so this was leaking reactflow into the eager chunks. Converted to import type — erased at compile time, keeps the lazy boundary clean. Affected files:

  • src/utils/EntityUtils.tsx (Node)
  • src/utils/EdgeStyleUtils.ts (Edge)
  • src/utils/NodeUtils.ts (Node)

EntityLineageUtils.tsx and CanvasUtils.ts use runtime values from reactflow (getBezierPath, MarkerType, Position.Left) and need value imports — left alone.

What's NOT in this PR

  • Splitting the 7.71 MB grab-bag further — it's still the biggest chunk. Contains lazy-loaded application code (Lineage, KnowledgeGraph, DataInsight, AsyncDeleteProvider) co-imported by Rollup's auto-chunking. Breaking it up requires adding manualChunks rules for app paths (src/pages/DataInsightPage, src/components/Lineage), which is a bigger surgery and warrants its own PR.
  • Audit of remaining reactflow value imports in lazy-only utils — those are fine because they're already on the lazy path. Future cleanup.

Type of change:

  • Improvement

Frontend Preview (Loom)

N/A — pure build-config / type-only change. Verified by:

  1. Baseline yarn build on main → captured chunk sizes
  2. Apply changes → yarn build → re-measure
  3. Confirm AsyncDeleteProvider shrunk and vendor-* chunks created with reactflow / recharts / rjsf / codemirror code isolated

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is in the form of <type>: <title> and follows Conventional Commits Specification
  • My commits follow the Conventional Commits Specification.
  • I have commented on my code, particularly in hard-to-understand areas.
  • N/A — pure build-config + type-only import change. No new tests required; existing tests cover the runtime behavior.
  • UI checkstyle passes locally on touched files.
  • yarn build succeeds and produces the expected vendor chunks.
  • I have updated the API documentation

🤖 Generated with Claude Code

…vendor chunks

Without explicit `manualChunks` rules Rollup's auto-chunking heuristic
lumped reactflow, recharts, rjsf, react-grid-layout and codemirror into
a single 8.87 MB grab-bag chunk (named after the first lazy entry to
pull them in — `AsyncDeleteProvider`). Two effects:
  * On any tab change, the browser had to re-download or re-parse a
    chunk full of code unrelated to the tab the user clicked.
  * No browser-level parallel fetch — one giant chunk instead of
    several smaller cacheable ones.

This adds named vendor chunks so each heavy lib lives in its own file:

  vendor-reactflow      94 KB raw / 27 KB brotli
  vendor-recharts      492 KB raw / 106 KB brotli
  vendor-rjsf          267 KB raw / 77 KB brotli
  vendor-codemirror    277 KB raw / 80 KB brotli

The grab-bag chunk shrunk 8.87 MB → 7.71 MB (-13%). Each vendor chunk
is independently cacheable across deploys and fetched in parallel.

`reactflow` is a meta-package re-exporting from `@reactflow/*` sub-
packages (core, background, controls, minimap, node-resizer, node-
toolbar). The rule matches both prefixes; matching only the meta-
package missed the bulk of the bytes and produced no chunk at all on
the first attempt.

Plus three small type-only import conversions where the symbol was
purely a type annotation:
  - utils/EntityUtils.tsx: `Node` (universal page-load util)
  - utils/EdgeStyleUtils.ts: `Edge`
  - utils/NodeUtils.ts: `Node`

These were value imports — even though `Node`/`Edge` are types in
reactflow's API, a value import drags reactflow's runtime into
whatever chunk imports the util. Type-only imports are erased at
compile time, keeping the lazy boundary clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@harshach harshach requested a review from a team as a code owner May 10, 2026 17:25
Copilot AI review requested due to automatic review settings May 10, 2026 17:25
@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels May 10, 2026
Comment on lines +221 to +226
if (
id.includes('node_modules/recharts') ||
id.includes('node_modules/d3-')
) {
return 'vendor-recharts';
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Performance: Grouping all d3-* into vendor-recharts creates unwanted coupling

The rule id.includes('node_modules/d3-') assigns all d3 sub-packages to vendor-recharts. However, @reactflow/core depends on d3-zoom, d3-selection, and d3-drag. In a typical hoisted node_modules layout, these resolve to node_modules/d3-zoom/... (not nested under @reactflow/), so they won't match the reactflow rule — they'll land in vendor-recharts.

This means loading the Lineage tab (which uses reactflow but not recharts) will force the browser to also download and parse vendor-recharts just for the three d3 modules reactflow needs. This partially defeats the goal of independent, parallel-fetchable chunks.

Suggested fix: Either create a separate vendor-d3 chunk for shared d3 packages, or narrow the recharts rule to only match recharts-specific d3 packages (d3-shape, d3-scale, d3-interpolate, d3-path, d3-color, d3-format, d3-time, d3-time-format, d3-array), leaving reactflow's d3 deps (d3-zoom, d3-selection, d3-drag, d3-transition, d3-dispatch, d3-ease, d3-timer) to either fall into vendor-reactflow via explicit matching or remain in a shared chunk.

Suggested fix:

if (
  id.includes('node_modules/recharts') ||
  id.includes('node_modules/d3-shape') ||
  id.includes('node_modules/d3-scale') ||
  id.includes('node_modules/d3-interpolate') ||
  id.includes('node_modules/d3-path') ||
  id.includes('node_modules/d3-array')
) {
  return 'vendor-recharts';
}
if (id.includes('node_modules/d3-')) {
  return 'vendor-d3';
}
  • Apply suggested fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 10, 2026

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Splits heavy dependencies into individual vendor chunks to improve parallel loading and cacheability, but the d3-* grouping within vendor-recharts creates unnecessary coupling.

⚠️ Performance: Grouping all d3-* into vendor-recharts creates unwanted coupling

📄 openmetadata-ui/src/main/resources/ui/vite.config.ts:221-226

The rule id.includes('node_modules/d3-') assigns all d3 sub-packages to vendor-recharts. However, @reactflow/core depends on d3-zoom, d3-selection, and d3-drag. In a typical hoisted node_modules layout, these resolve to node_modules/d3-zoom/... (not nested under @reactflow/), so they won't match the reactflow rule — they'll land in vendor-recharts.

This means loading the Lineage tab (which uses reactflow but not recharts) will force the browser to also download and parse vendor-recharts just for the three d3 modules reactflow needs. This partially defeats the goal of independent, parallel-fetchable chunks.

Suggested fix: Either create a separate vendor-d3 chunk for shared d3 packages, or narrow the recharts rule to only match recharts-specific d3 packages (d3-shape, d3-scale, d3-interpolate, d3-path, d3-color, d3-format, d3-time, d3-time-format, d3-array), leaving reactflow's d3 deps (d3-zoom, d3-selection, d3-drag, d3-transition, d3-dispatch, d3-ease, d3-timer) to either fall into vendor-reactflow via explicit matching or remain in a shared chunk.

Suggested fix
if (
  id.includes('node_modules/recharts') ||
  id.includes('node_modules/d3-shape') ||
  id.includes('node_modules/d3-scale') ||
  id.includes('node_modules/d3-interpolate') ||
  id.includes('node_modules/d3-path') ||
  id.includes('node_modules/d3-array')
) {
  return 'vendor-recharts';
}
if (id.includes('node_modules/d3-')) {
  return 'vendor-d3';
}
🤖 Prompt for agents
Code Review: Splits heavy dependencies into individual vendor chunks to improve parallel loading and cacheability, but the `d3-*` grouping within `vendor-recharts` creates unnecessary coupling.

1. ⚠️ Performance: Grouping all d3-* into vendor-recharts creates unwanted coupling
   Files: openmetadata-ui/src/main/resources/ui/vite.config.ts:221-226

   The rule `id.includes('node_modules/d3-')` assigns **all** d3 sub-packages to `vendor-recharts`. However, `@reactflow/core` depends on `d3-zoom`, `d3-selection`, and `d3-drag`. In a typical hoisted `node_modules` layout, these resolve to `node_modules/d3-zoom/...` (not nested under `@reactflow/`), so they won't match the reactflow rule — they'll land in `vendor-recharts`.
   
   This means loading the Lineage tab (which uses reactflow but not recharts) will force the browser to also download and parse `vendor-recharts` just for the three d3 modules reactflow needs. This partially defeats the goal of independent, parallel-fetchable chunks.
   
   **Suggested fix:** Either create a separate `vendor-d3` chunk for shared d3 packages, or narrow the recharts rule to only match recharts-specific d3 packages (`d3-shape`, `d3-scale`, `d3-interpolate`, `d3-path`, `d3-color`, `d3-format`, `d3-time`, `d3-time-format`, `d3-array`), leaving reactflow's d3 deps (`d3-zoom`, `d3-selection`, `d3-drag`, `d3-transition`, `d3-dispatch`, `d3-ease`, `d3-timer`) to either fall into `vendor-reactflow` via explicit matching or remain in a shared chunk.

   Suggested fix:
   if (
     id.includes('node_modules/recharts') ||
     id.includes('node_modules/d3-shape') ||
     id.includes('node_modules/d3-scale') ||
     id.includes('node_modules/d3-interpolate') ||
     id.includes('node_modules/d3-path') ||
     id.includes('node_modules/d3-array')
   ) {
     return 'vendor-recharts';
   }
   if (id.includes('node_modules/d3-')) {
     return 'vendor-d3';
   }

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves frontend perceived latency by making Vite/Rollup emit separate, named vendor chunks for several heavy tab-specific libraries (reactflow, recharts, rjsf, codemirror, react-grid-layout), keeping lazy boundaries effective and enabling better browser caching/parallel fetch. It also prevents accidental eager-bundle inclusion of reactflow by converting a few type-only imports to import type.

Changes:

  • Add explicit manualChunks rules in Vite Rollup config to split heavy libraries into dedicated vendor-* chunks.
  • Convert reactflow Node/Edge imports to type-only imports in utility modules that are on (or can be on) the eager path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
openmetadata-ui/src/main/resources/ui/vite.config.ts Adds manualChunks rules to force heavy dependencies into separate vendor chunks.
openmetadata-ui/src/main/resources/ui/src/utils/NodeUtils.ts Switches reactflow Node import to import type to avoid runtime bundling.
openmetadata-ui/src/main/resources/ui/src/utils/EntityUtils.tsx Switches reactflow Node import to import type to keep reactflow off the eager path.
openmetadata-ui/src/main/resources/ui/src/utils/EdgeStyleUtils.ts Switches reactflow Edge import to import type to avoid dragging reactflow into importing chunks.

@github-actions
Copy link
Copy Markdown
Contributor

🔴 Playwright Results — 12 failure(s)

✅ 0 passed · ❌ 12 failed · 🟡 0 flaky · ⏭️ 4159 skipped

Shard Passed Failed Flaky Skipped
🔴 Shard 1 0 2 0 301
🔴 Shard 2 0 2 0 774
🔴 Shard 3 0 2 0 786
🔴 Shard 4 0 2 0 806
🔴 Shard 5 0 2 0 748
🔴 Shard 6 0 2 0 744

Genuine Failures (failed on all attempts)

entity-data.teardown.ts › cleanup entity data prerequisites (shard 1)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 1)
�[31mTest timeout of 120000ms exceeded.�[39m
entity-data.teardown.ts › cleanup entity data prerequisites (shard 2)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 2)
�[31mTest timeout of 120000ms exceeded.�[39m
entity-data.teardown.ts › cleanup entity data prerequisites (shard 3)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 3)
�[31mTest timeout of 120000ms exceeded.�[39m
entity-data.teardown.ts › cleanup entity data prerequisites (shard 4)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 4)
�[31mTest timeout of 120000ms exceeded.�[39m
entity-data.teardown.ts › cleanup entity data prerequisites (shard 5)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 5)
�[31mTest timeout of 120000ms exceeded.�[39m
entity-data.teardown.ts › cleanup entity data prerequisites (shard 6)
�[31mTest timeout of 300000ms exceeded.�[39m
auth.setup.ts › authenticate all users (shard 6)
�[31mTest timeout of 120000ms exceeded.�[39m

📦 Download artifacts

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

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

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants