-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
meta: remove weird fetch/generator thingies, rely on next's internal revalidate api #8001
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 Git ↗︎
|
Lighthouse Results
|
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 removes the obsolete self-fetching mechanism that was used to retrieve data through internal Next.js APIs, simplifying the architecture by relying on Next.js's built-in revalidation instead. The changes eliminate hacky fetch operations that were problematic across different deployment environments.
- Removes internal fetch-based data retrieval system and associated API routes
- Converts async React Server Components to synchronous components
- Eliminates environment-specific logic for VERCEL_REGION and related constants
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
turbo.json | Removes VERCEL_REGION environment variable from build configurations |
next.constants.mjs | Removes VERCEL_REGION, IS_NOT_VERCEL_RUNTIME_ENV, and NEXT_DATA_URL constants |
next-data/releaseData.ts | Removes entire fetch-based release data retrieval module |
next-data/downloadSnippets.ts | Removes entire fetch-based download snippets retrieval module |
layouts/Download.tsx | Converts async component to synchronous |
layouts/Blog.tsx | Converts from server-side to client-side translations and removes async |
components/withNodeRelease.tsx | Converts async component to synchronous, uses direct provider import |
components/withDownloadSection.tsx | Converts async component to synchronous, uses direct provider imports |
app/[locale]/next-data/release-data/route.ts | Removes API route entirely |
app/[locale]/next-data/download-snippets/route.tsx | Removes API route entirely |
cc @nodejs/nodejs-website this is a cleanup, would appreciate some 👀 no need to fast-track. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #8001 +/- ##
==========================================
- Coverage 73.04% 72.92% -0.12%
==========================================
Files 95 95
Lines 8341 8308 -33
Branches 217 215 -2
==========================================
- Hits 6093 6059 -34
- Misses 2247 2248 +1
Partials 1 1 ☔ View full report in Codecov by Sentry. |
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.
LGTM !
Signed-off-by: Claudio Wunder <[email protected]>
dee9ac4
to
6fded37
Compare
@ovflowd Requesting fast-track |
cc @nodejs/nodejs-website requesting fast-track as this apparently fixes the |
This PR removes our obsolete hacky approach of fetching() stuff on certain environments. Due to the staticness of our website (our dynamic router is static) and revalidates data as needed. These APIs were introduced at a time we were not using such feature.