-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
fix(nextjs): fix static exports #8439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8439 +/- ##
==========================================
+ Coverage 73.59% 73.66% +0.06%
==========================================
Files 108 108
Lines 9193 9193
Branches 313 312 -1
==========================================
+ Hits 6766 6772 +6
+ Misses 2425 2419 -6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
bjohansebas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change the CI workflow to use the deploy command, please
There was a problem hiding this 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 fixes Next.js static exports by replacing the experimental use cache directive with React's cache() function and removing unnecessary use server directives. The changes enable proper static site generation by moving data generation to module initialization time rather than runtime.
Key changes:
- Removed experimental
useCache: trueflag from Next.js configuration - Converted provider files to use React's
cache()function with top-level await for data generation - Removed
use serverdirectives and updated components to call synchronous cached provider functions
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/site/next.config.mjs | Removed experimental useCache flag |
| apps/site/next-env.d.ts | Updated types reference path for development environment |
| apps/site/next-data/providers/vulnerabilities.ts | Converted from use cache directive to React cache() with top-level data generation |
| apps/site/next-data/providers/supportersData.ts | Converted from use cache directive to React cache() with top-level data generation |
| apps/site/next-data/providers/releaseVersions.ts | Converted from use cache directive to React cache() with top-level data generation |
| apps/site/next-data/providers/releaseData.ts | Converted from use cache directive to React cache() with top-level data generation |
| apps/site/next-data/providers/downloadSnippets.ts | Converted from use cache directive to React cache() with top-level data generation |
| apps/site/layouts/Download.tsx | Removed await from provider call |
| apps/site/components/withSupporters.tsx | Removed use server directive and async/await from component |
| apps/site/components/withReleaseSelect.tsx | Removed use server directive and await from provider call |
| apps/site/components/withNodeRelease.tsx | Removed use server directive and async/await from component |
| apps/site/components/withDownloadSection.tsx | Removed await from provider calls |
| apps/site/components/withDownloadArchive.tsx | Removed async/await from component and provider call |
| apps/site/components/Releases/PreviousReleasesTable/index.tsx | Removed await from provider call |
| apps/site/components/EOL/EOLReleaseTable/index.tsx | Removed await from provider calls |
| apps/site/app/[locale]/not-found.tsx | Removed use server directive |
| apps/site/app/[locale]/download/archive/[version]/page.tsx | Removed async/await from generateStaticParams and provider calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Can we make sure the PR CI is also updated to do a static build? |
📦 Build Size ComparisonSummary
Changes➕ Added Assets (9)
➖ Removed Assets (9)
|
609ea30 to
1fedc01
Compare
Co-authored-by: Claudio Wunder <[email protected]> Signed-off-by: Aviv Keller <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM! Can you just double check at the very least these cache() inner functions are being executed once per build-worker instead of once per every page generated? 🤔 (The difference is having 4-8 console logs instead of 2K+)
|
Yes, it's only being run once per build worker. Only saw a couple of logs on the static build, not for every page |
That's already a much better outcome, because the top-level await would be executed for every frigging page if I recall correctly. Okay, let's fast-track this PR, cc @nodejs/web-infra |
….org into fix-static-exports
As discussed on Slack, this PR fixes static exports by reworking
use cacheanduse serverfiles.