Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a breaking change introduced by a recent rolldown upgrade by refactoring the server-side rendering code generation approach. Instead of generating unique variable names and capturing results, the code now directly returns values from the generated functions.
- Removed the dynamic SSR variable generation mechanism
- Updated
buildServerProgramto use direct return statements instead of variable assignment - Simplified
executeServerCodeto work with the new return-based approach
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/generators/web/utils/processing.mjs | Removed SSR variable generation and updated function calls to match new API |
| src/generators/web/utils/generate.mjs | Modified buildServerProgram to return values directly instead of assigning to variables |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #395 +/- ##
==========================================
+ Coverage 74.27% 74.30% +0.03%
==========================================
Files 118 118
Lines 11041 11031 -10
Branches 695 695
==========================================
- Hits 8201 8197 -4
+ Misses 2837 2831 -6
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Why did no CI catch this? Can we make sure we have whatever we need in CI so we can trust dependency upgrades? |
My assumption is that the script did fail (on Vercel), but was deployed despite the failure. We can prevent this from happening again by either:
|
|
👍 Can you make sure an issue is cut for that, I'd vote for doing both. |
MattIPv4
left a comment
There was a problem hiding this comment.
I do not claim to know what's happening in this logic, but the diff seems logical
For the record, in the rolldown update, tree-shaking was updated to shake unused variables, but no longer shake unused returns, so our logic (which previously accounted for shaked returns via variables) needed to be flipped |
@nodejs/web-infra Requesting fast-track.
The recent rolldown upgrade performed by Dependabot contained a breaking change. This PR updates our codebase to work with the new rolldown version.