infra: performance improvements #488
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #488 +/- ##
==========================================
- Coverage 74.42% 74.10% -0.33%
==========================================
Files 110 111 +1
Lines 10468 10562 +94
Branches 700 701 +1
==========================================
+ Hits 7791 7827 +36
- Misses 2674 2732 +58
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@avivkeller thoughts? |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the web generator to support batch processing of multiple JSX entries and implements performance optimizations through safe file operations.
- Introduces
safeWriteandsafeCopyutilities to avoid unnecessary file writes by comparing file sizes - Refactors bundling to process multiple entry points together, enabling code splitting and import maps
- Updates Rolldown from beta.40 to beta.47 and changes output format from IIFE to ESM with module support
Reviewed Changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/safeWrite.mjs | New utility to conditionally write files based on size comparison |
| src/utils/safeCopy.mjs | New utility for efficient file copying with change detection |
| src/utils/tests/safeCopy.test.mjs | Comprehensive test suite for safeCopy utility |
| src/generators/web/utils/processing.mjs | Refactored to process multiple JSX entries in batch, now returns results array with js chunks |
| src/generators/web/utils/bundle.mjs | Enhanced to support multiple entry points, ESM output, code splitting, and import maps |
| src/generators/web/ui/index.css | Added @node-core/rehype-shiki CSS import |
| src/generators/web/ui/components/CodeBox.jsx | Updated to pass className and children to BaseCodeBox, fixed notification duration |
| src/generators/web/template.html | Updated to use ES modules and import maps instead of IIFE |
| src/generators/web/index.mjs | Refactored to use batch processing and safeWrite for all file operations |
| src/generators/legacy-html/index.mjs | Updated import path for safeCopy utility |
| package.json | Added @shikijs/twoslash dependency and upgraded Rolldown to beta.47 |
| npm-shrinkwrap.json | Lockfile updates for dependency changes |
Files not reviewed (1)
- npm-shrinkwrap.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
This PR does a quite a lot of work, and it's hard to review what's related to performance and what's not. Can we break it up into several PRs? A lot of changes seem unrelated (ie all the comment changes)
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
I'm gonna put a blocker on this for now, since it's really hard to review this many changes.
Can we try removing all of the rewritten comments, and then breaking this into what's perf related and what's not?
Unfortunately that was the AI-nonsense. I asked it to update tests and comments and it started spiralling out of control. That's what happens when I try to multitask, apologies. |
avivkeller
left a comment
avivkeller
left a comment
There was a problem hiding this comment.
I am no longer blocking, but I still have (a few) comments
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- npm-shrinkwrap.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
😆 |
This PR optimizes our build process by processing all entries at time and chunking our client-bundle and using Rolldown's importMap feature.