Skip to content

perf(web): filter codemirror chunk out of <modulepreload>#115

Merged
naorsabag merged 1 commit into
masterfrom
perf/filter-codemirror-preload
May 10, 2026
Merged

perf(web): filter codemirror chunk out of <modulepreload>#115
naorsabag merged 1 commit into
masterfrom
perf/filter-codemirror-preload

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 10, 2026

Re-ran Lighthouse after #114 deployed — Performance barely moved (mobile 42 → 41, desktop 89 → 84), even though the chunk split looked correct. Inspecting the deployed `index.html` showed why:

```html

\`\`\`

Vite/rolldown was eagerly emitting `modulepreload` for every named chunk, including the lazy codemirror one. Browsers happily fetched, parsed, and prepared CodeMirror on first paint anyway — exactly what the React.lazy on FlowEditorModal was supposed to prevent.

Fix: `build.modulePreload.resolveDependencies` strips the codemirror chunk from the preload list. The chunk still ships and still loads when FlowEditorModal's dynamic import runs (on "+ New flow" / Edit click), just not before then.

What's preloaded after this PR

Before:
```
rolldown-runtime, codemirror, flow, yaml, react
```

After:
```
rolldown-runtime, flow, yaml, react
```

That's ~140 KB gz off the first-paint network + parse budget.

SEO win from #114 stands

SEO went from 91 → 100 already; this PR doesn't touch it.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Performance
    • Optimized the code editor component loading to reduce initial page load time by deferring non-critical resources until they are actually needed.

Review Change Stack

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>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 10, 2026

Walkthrough

This PR modifies the Vite build configuration to add a build.modulePreload.resolveDependencies hook that filters out modulepreload links for CodeMirror chunks, ensuring they are only fetched when dynamically imported rather than eagerly preloaded at startup.

Changes

CodeMirror Lazy Loading Configuration

Layer / File(s) Summary
Build modulepreload configuration
packages/web/vite.config.ts
The build.modulePreload.resolveDependencies hook removes CodeMirror chunk preloads from the dependency list to prevent eager preloading of lazy-imported modules.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • naorsabag/openhop#114: Both PRs handle CodeMirror code splitting; this PR filters modulepreload while the related PR adds manual chunk splitting for the codemirror module.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: filtering the CodeMirror chunk from modulepreload links in the Vite configuration for performance optimization.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/filter-codemirror-preload

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/vite.config.ts (1)

42-44: ⚡ Quick win

Scope this filter to HTML preloads only.

At Line 42, the filter is unconditional and will remove codemirror dependency preloads for both HTML and dynamic JS imports. This can add a waterfall when opening the editor. Restrict the filter to HTML host preloads using the context.hostType parameter, which preserves lazy-load responsiveness for dynamic imports while keeping first-paint optimizations.

♻️ Proposed patch
-      resolveDependencies(_filename, deps) {
-        return deps.filter((d) => !/\/codemirror-[A-Za-z0-9_-]+\.js$/.test(d))
+      resolveDependencies(_filename, deps, context) {
+        if (context.hostType !== 'html') return deps
+        return deps.filter((d) => !/(?:^|\/)codemirror-[A-Za-z0-9_-]+\.js$/.test(d))
       },
🤖 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/vite.config.ts` around lines 42 - 44, The unconditional filter
in resolveDependencies removes codemirror preloads for all host types; change
resolveDependencies(_filename, deps, context) to only filter when
context?.hostType === 'html' so HTML preloads drop codemirror-*.js but
dynamic/js imports keep those deps for lazy loading; specifically update the
resolveDependencies function to check context.hostType and return deps unchanged
for non-'html' hosts and apply the existing regex filter only for HTML hostType.
🤖 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/vite.config.ts`:
- Around line 42-44: The unconditional filter in resolveDependencies removes
codemirror preloads for all host types; change resolveDependencies(_filename,
deps, context) to only filter when context?.hostType === 'html' so HTML preloads
drop codemirror-*.js but dynamic/js imports keep those deps for lazy loading;
specifically update the resolveDependencies function to check context.hostType
and return deps unchanged for non-'html' hosts and apply the existing regex
filter only for HTML hostType.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3173a369-7b32-46a4-91b8-84583bf3d235

📥 Commits

Reviewing files that changed from the base of the PR and between e3f9236 and b69f4d3.

📒 Files selected for processing (1)
  • packages/web/vite.config.ts

@naorsabag naorsabag merged commit c502a9e into master May 10, 2026
8 checks passed
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/filter-codemirror-preload 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