Skip to content

perf(web): lazy editor + vendor-split + meta description#114

Merged
naorsabag merged 1 commit into
masterfrom
perf/lazy-editor-and-vendor-split
May 10, 2026
Merged

perf(web): lazy editor + vendor-split + meta description#114
naorsabag merged 1 commit into
masterfrom
perf/lazy-editor-and-vendor-split

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 10, 2026

Lighthouse mobile scored 42/100 on https://naorsabag.github.io/openhop/ (full report below). Largest culprit: the build shipped a single 2.5 MB / 768 KB gz bundle on first paint, with ~2.4 s of unused JavaScript. Three changes here.

1. Lazy-load FlowEditorModal

`CodeMirror` + `@codemirror/lang-yaml` is the heaviest dependency that isn't on the critical path. `React.lazy` + mount-on-open means the chunk only fetches on the first "+ New flow" / Edit click.

2. Vite manualChunks

Function form (rolldown rejects the object form rollup used). New chunk layout:

chunk gz when
`index` (entry) 45 KB always
`react` 56 KB always
`flow` (`@xyflow/react`, `elkjs`) 493 KB always
`yaml` (`yaml`, `lz-string`) 32 KB always
`codemirror` 140 KB lazy — only when editor opens
`FlowEditorModal` 2 KB lazy — only when editor opens

Initial download drops from 768 KB → ~626 KB gz, and most of the saving is on the executed main-thread JS, where Lighthouse penalty is biggest.

3. `` + ``

Fixes the SEO miss (`Document does not have a meta description`) and gives mobile the green address-bar tint that matches the canvas background.

Lighthouse before

Mobile: Performance 42 · A11y 96 · BP 100 · SEO 91. FCP 5.5 s · LCP 6.5 s · TBT 980 ms · TTI 7.4 s.
Desktop: Performance 89 · A11y 93 · BP 100 · SEO 91. FCP 1.3 s · LCP 1.4 s.

Test plan

  • `npm test -w @openhop/web` (42/42)
  • `VITE_BASE=/openhop/ VITE_FRAGMENT_MODE=1 npm run build` — chunks split as listed above
  • After Pages redeploys: rerun Lighthouse, confirm Perf improves and SEO hits 100

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Performance

    • Optimized asset loading for faster initial page load times
    • Enhanced code splitting to improve browser caching and overall performance
  • Documentation

    • Updated site metadata for improved search engine discoverability

Review Change Stack

Lighthouse mobile scored 42/100 on Pages, largely because the build
shipped a single 2.5 MB / 768 KB gz bundle. Three changes here:

1. Lazy-load FlowEditorModal in both App.tsx and AppFragment.tsx.
   CodeMirror + the YAML language pack is the heaviest dependency
   that isn't used until the user opens the editor. React.lazy +
   mount-on-open means the chunk only fetches on first
   "+ New flow" / Edit click.

2. Vite manualChunks (function form — rolldown doesn't accept the
   object form) splits the entry bundle into:
     react         — react, react-dom, scheduler          (56 KB gz)
     flow          — @xyflow/react, elkjs                (493 KB gz)
     codemirror    — @uiw/react-codemirror, @codemirror/* (140 KB gz, LAZY)
     yaml          — yaml, lz-string                       (32 KB gz)
     FlowEditorModal — app-side wrapper                     (2 KB gz, LAZY)
     index (entry) — everything else                       (45 KB gz)
   First paint drops from one 768 KB request to several smaller
   parallel ones, and codemirror is off the critical path entirely.

3. <meta name="description"> + <meta name="theme-color"> in
   index.html — fixes the Lighthouse SEO miss and gives mobile the
   green address-bar tint that matches the canvas.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Walkthrough

The pull request optimizes the FlowEditorModal component by converting it from a statically imported, always-mounted component to a lazy-loaded component that only loads and mounts when the editor is open. The Vite build configuration is updated to chunk vendor dependencies by category, reducing the initial bundle size.

Changes

FlowEditorModal Lazy Loading and Build Optimization

Layer / File(s) Summary
Metadata Update
packages/web/index.html
HTML head now includes a meta description tag describing OpenHop.
Lazy Loading Setup
packages/web/src/App.tsx, packages/web/src/AppFragment.tsx
FlowEditorModal is imported via React.lazy with dynamic import instead of static import in both component files.
Conditional Mounting with Suspense
packages/web/src/App.tsx, packages/web/src/AppFragment.tsx
FlowEditorModal is conditionally rendered only when editor.mode !== 'closed' and wrapped in a Suspense boundary with null fallback, replacing the previous always-mounted pattern with open prop control.
Build Chunk Configuration
packages/web/vite.config.ts
Vite config adds build.rollupOptions.output.manualChunks to split vendor dependencies into named chunks (flow, codemirror, yaml, react) based on module ID paths.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • naorsabag/openhop#76: Introduces the FlowEditorModal component that is now being lazy-loaded in this PR, with direct dependency on the lazy-loading and chunking changes.
  • naorsabag/openhop#107: Also modifies AppFragment.tsx with UI changes, overlapping the same component file being refactored for lazy loading here.
  • naorsabag/openhop#77: Related changes to AppFragment and FlowEditorModal component structure and behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title precisely captures the three key changes: lazy loading the editor modal, vendor code splitting, and adding meta description to improve performance and SEO.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/lazy-editor-and-vendor-split

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/web/src/App.tsx (1)

546-547: ⚡ Quick win

Use a visible Suspense fallback for modal-open feedback.

fallback={null} can make the “New flow / Edit” click appear to do nothing while the chunk downloads.

Suggested tweak
-      {editor.mode !== 'closed' && (
-        <Suspense fallback={null}>
+      {editor.mode !== 'closed' && (
+        <Suspense
+          fallback={
+            <div className="fixed inset-0 z-[1100] grid place-items-center bg-black/40 font-terminal text-text/70">
+              Loading editor…
+            </div>
+          }
+        >
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/App.tsx` around lines 546 - 547, The Suspense wrapper around
FlowEditorModal uses fallback={null}, which gives no UI feedback when the modal
chunk is loading; change the Suspense to render a visible fallback (e.g., a
small spinner or loading overlay component) instead of null so users see
immediate feedback when they click "New flow / Edit". Locate the Suspense that
wraps FlowEditorModal (symbol: Suspense and FlowEditorModal) and replace the
null fallback with your app's loading indicator component (or a simple
accessible spinner with an aria-label) so the modal open action shows a visible
loading state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/web/src/App.tsx`:
- Around line 546-547: The Suspense wrapper around FlowEditorModal uses
fallback={null}, which gives no UI feedback when the modal chunk is loading;
change the Suspense to render a visible fallback (e.g., a small spinner or
loading overlay component) instead of null so users see immediate feedback when
they click "New flow / Edit". Locate the Suspense that wraps FlowEditorModal
(symbol: Suspense and FlowEditorModal) and replace the null fallback with your
app's loading indicator component (or a simple accessible spinner with an
aria-label) so the modal open action shows a visible loading state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4947cfa1-8366-4983-b0d8-24e7bc01f8f6

📥 Commits

Reviewing files that changed from the base of the PR and between 25d79ff and c007065.

📒 Files selected for processing (4)
  • packages/web/index.html
  • packages/web/src/App.tsx
  • packages/web/src/AppFragment.tsx
  • packages/web/vite.config.ts

@naorsabag naorsabag merged commit e3f9236 into master May 10, 2026
8 checks passed
naorsabag added a commit that referenced this pull request May 10, 2026
The previous perf PR (#114) lazy-loaded FlowEditorModal but Lighthouse
showed no real win. Inspecting the deployed index.html revealed why:

    <link rel="modulepreload" href="/openhop/assets/codemirror-*.js">

Vite/rolldown was eagerly emitting modulepreload for *every* named
chunk, including the lazy codemirror one — which made browsers fetch
and parse CodeMirror on first paint anyway, defeating the React.lazy.

Add build.modulePreload.resolveDependencies that strips the
codemirror chunk from the preload list. The chunk still ships and
still loads when FlowEditorModal's dynamic import runs, just not
before then.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
naorsabag added a commit that referenced this pull request May 10, 2026
…test

- @openhop/server: 0.1.0-beta.2 → 0.1.0
- @openhop/web:    0.1.0-beta.4 → 0.1.0
- openhop CLI:     0.1.0-beta.6 → 0.1.0
  + bump pinned @openhop/server, @openhop/web deps + hardcoded --version

publish.yml: drop `--tag beta` from all three publish steps. With no
--tag, `npm publish` sets the `latest` dist-tag — so npmjs.com pages
and `npm install <pkg>` (no @beta suffix) automatically reflect the
new version on every successful publish. No more manual
`npm dist-tag add latest` step.

Bundles 13 PRs since v0.1.0-beta.6:

Web (heavy):
- #106 sprite URLs use Vite BASE_URL (Pages 404 fix)
- #107 example flow cards on empty state
- #108 sidebar + carrot-click highlight for string-data steps
- #109 all 5 examples grouped under examples/ + first-visit autoload
- #112 per-step auto-zoom + playback speed button
- #113 collapse FLOWS / INSPECT by default on mobile
- #114 lazy editor + vendor-split chunks + meta description
- #115 filter codemirror from <modulepreload>
- #117 memo FlowNode + GPU-layer sprites for smoother pan/zoom

CLI:
- #116 OpenSkills fallback hint when no Tier-1 client gets the skill
- #118 suppress the hint when Tier-1 already has the skill

Server: untouched code; bumped to keep the 0.1.0 set consistent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
naorsabag added a commit that referenced this pull request May 11, 2026
…test (#119)

- @openhop/server: 0.1.0-beta.2 → 0.1.0
- @openhop/web:    0.1.0-beta.4 → 0.1.0
- openhop CLI:     0.1.0-beta.6 → 0.1.0
  + bump pinned @openhop/server, @openhop/web deps + hardcoded --version

publish.yml: drop `--tag beta` from all three publish steps. With no
--tag, `npm publish` sets the `latest` dist-tag — so npmjs.com pages
and `npm install <pkg>` (no @beta suffix) automatically reflect the
new version on every successful publish. No more manual
`npm dist-tag add latest` step.

Bundles 13 PRs since v0.1.0-beta.6:

Web (heavy):
- #106 sprite URLs use Vite BASE_URL (Pages 404 fix)
- #107 example flow cards on empty state
- #108 sidebar + carrot-click highlight for string-data steps
- #109 all 5 examples grouped under examples/ + first-visit autoload
- #112 per-step auto-zoom + playback speed button
- #113 collapse FLOWS / INSPECT by default on mobile
- #114 lazy editor + vendor-split chunks + meta description
- #115 filter codemirror from <modulepreload>
- #117 memo FlowNode + GPU-layer sprites for smoother pan/zoom

CLI:
- #116 OpenSkills fallback hint when no Tier-1 client gets the skill
- #118 suppress the hint when Tier-1 already has the skill

Server: untouched code; bumped to keep the 0.1.0 set consistent.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@naorsabag naorsabag deleted the perf/lazy-editor-and-vendor-split branch May 15, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant