⚡ Bolt: Parallelize generateStaticParams fetching across years#210
⚡ Bolt: Parallelize generateStaticParams fetching across years#210anyulled wants to merge 1 commit into
Conversation
Replaced sequential for...of loops with Promise.all + array.map in the generateStaticParams functions across dynamic route pages. This speeds up the Next.js build time by fetching data for multiple editions (years) concurrently rather than waiting for each year to finish before starting the next one. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughFour dynamic route pages ( ChangesConcurrent Static Params Generation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the generateStaticParams function across several dynamic routes to utilize Promise.all, allowing for concurrent data fetching across different years. While this improves build performance, the review feedback points out that the implementation swallows errors within try...catch blocks. This prevents the build from failing if data fetching fails, potentially leading to the deployment of incomplete pages. The reviewer recommends using a strict mode for data fetching to ensure build integrity.
| years.map(async (year) => { | ||
| try { | ||
| const companies = await getJobOffersForEdition(year); | ||
| return companies.map((company) => ({ | ||
| year, | ||
| companyName: company.id, | ||
| })); | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch job offers for year ${year}:`, error); | ||
| return []; | ||
| } | ||
| }) |
There was a problem hiding this comment.
Swallowing errors in generateStaticParams prevents the build from failing when data fetching fails, which can lead to the deployment of incomplete pages. According to the general rules, data fetching during the build process should fail the build on error to ensure site integrity.
years.map(async (year) => {
const companies = await getJobOffersForEdition(year);
return companies.map((company) => ({
year,
companyName: company.id,
}));
})
References
- Data fetching functions used during the build process should include a 'strict' mode that throws an error on failure to ensure the build fails and prevents deployment of incomplete pages.
| years.map(async (year) => { | ||
| try { | ||
| const speakers = await getSpeakers(year); | ||
| return speakers.map((speaker) => ({ year, speakerId: speaker.id })); | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch speakers for year ${year}:`, error); | ||
| return []; | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch speakers for year ${year}:`, error); | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Swallowing errors in generateStaticParams prevents the build from failing when data fetching fails. Additionally, getSpeakers supports a strict mode that should be enabled here to ensure the build fails if data cannot be retrieved, preventing the deployment of incomplete pages.
years.map(async (year) => {
const speakers = await getSpeakers(year, { strict: true });
return speakers.map((speaker) => ({ year, speakerId: speaker.id }));
})
References
- Data fetching functions used during the build process should include a 'strict' mode that throws an error on failure to ensure the build fails and prevents deployment of incomplete pages.
| years.map(async (year) => { | ||
| try { | ||
| const sessionGroups = await getTalks(year); | ||
| const allTalks = sessionGroups.flatMap((group) => group.sessions); | ||
| const allTags = new Set<string>(); | ||
|
|
||
| for (const talk of allTalks) { | ||
| getTagsFromTalk(talk).forEach((tag) => allTags.add(tag)); | ||
| } | ||
|
|
||
| return Array.from(allTags).map((tag) => ({ | ||
| year, | ||
| tag: tag.replaceAll(" ", "-").toLowerCase(), | ||
| })); | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch talks for year ${year}:`, error); | ||
| return []; | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch talks for year ${year}:`, error); | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Swallowing errors in generateStaticParams prevents the build from failing when data fetching fails. getTalks supports a strict mode that should be used here to ensure the build fails on error, avoiding the deployment of a site with missing tag pages.
years.map(async (year) => {
const sessionGroups = await getTalks(year, { strict: true });
const allTalks = sessionGroups.flatMap((group) => group.sessions);
const allTags = new Set<string>();
for (const talk of allTalks) {
getTagsFromTalk(talk).forEach((tag) => allTags.add(tag));
}
return Array.from(allTags).map((tag) => ({
year,
tag: tag.replaceAll(" ", "-").toLowerCase(),
}));
})
References
- Data fetching functions used during the build process should include a 'strict' mode that throws an error on failure to ensure the build fails and prevents deployment of incomplete pages.
| years.map(async (year) => { | ||
| try { | ||
| const sessionGroups = await getTalks(year); | ||
| const allTalks = sessionGroups.flatMap((group) => group.sessions); | ||
| return allTalks.map((talk) => ({ year, talkId: talk.id })); | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch talks for year ${year}:`, error); | ||
| return []; | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Failed to fetch talks for year ${year}:`, error); | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
Swallowing errors in generateStaticParams prevents the build from failing when data fetching fails. getTalks supports a strict mode that should be enabled here to ensure build failure on fetch errors, preventing incomplete static generation.
years.map(async (year) => {
const sessionGroups = await getTalks(year, { strict: true });
const allTalks = sessionGroups.flatMap((group) => group.sessions);
return allTalks.map((talk) => ({ year, talkId: talk.id }));
})
References
- Data fetching functions used during the build process should include a 'strict' mode that throws an error on failure to ensure the build fails and prevents deployment of incomplete pages.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/[year]/job-offers/[companyName]/page.tsx (1)
32-35: ⚖️ Poor tradeoffDocument the partial generation strategy for error resilience in
generateStaticParams.The consistent graceful degradation pattern (catch error, warn, return empty array) across these four functions allows partial static generation when some year fetches fail. This is a valid resilience strategy but should be explicitly documented. Either:
- Add a comment explaining why partial success is acceptable (e.g., "Graceful degradation: failed years skip page generation rather than blocking the entire build")
- Or reconsider whether failed data fetches should propagate and fail the build
Ensure the choice is intentional and documented consistently across all four files affected.
🤖 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 `@app/`[year]/job-offers/[companyName]/page.tsx around lines 32 - 35, The catch block in generateStaticParams (the one that logs "Failed to fetch job offers for year ${year}" and returns []) implements a graceful-degradation strategy that should be explicitly documented; update the generateStaticParams function to add a short comment above the try/catch explaining that partial static generation is intentional (e.g., "Graceful degradation: failed years skip page generation rather than blocking the entire build") and apply the same comment pattern to the other three files with identical catch+return [] behavior, or if you prefer failing the build change the catch to rethrow the error instead—ensure the chosen approach is documented consistently across all four implementations.
🤖 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 `@app/`[year]/job-offers/[companyName]/page.tsx:
- Around line 32-35: The catch block in generateStaticParams (the one that logs
"Failed to fetch job offers for year ${year}" and returns []) implements a
graceful-degradation strategy that should be explicitly documented; update the
generateStaticParams function to add a short comment above the try/catch
explaining that partial static generation is intentional (e.g., "Graceful
degradation: failed years skip page generation rather than blocking the entire
build") and apply the same comment pattern to the other three files with
identical catch+return [] behavior, or if you prefer failing the build change
the catch to rethrow the error instead—ensure the chosen approach is documented
consistently across all four implementations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca8b1d8a-d15e-488f-a940-475ec3ae50b9
📒 Files selected for processing (4)
app/[year]/job-offers/[companyName]/page.tsxapp/[year]/speakers/[speakerId]/page.tsxapp/[year]/tags/[tag]/page.tsxapp/[year]/talks/[talkId]/page.tsx
💡 What:
Replaced the sequential
for...ofloops used for generating static params in four Next.js dynamic routes (speakers/[speakerId],talks/[talkId],tags/[tag], andjob-offers/[companyName]) with a mapped approach usingPromise.all.🎯 Why:
The previous implementation fetched data for one year at a time, sequentially blocking the iteration. This meant that the total fetch time for static parameter generation was the sum of fetch times for every single year. As the number of past editions (years) grows, this becomes an increasingly severe bottleneck during Next.js builds.
📊 Impact:
By parallelizing the requests, Next.js can resolve and generate all the paths much faster. The total build time devoted to generating these static parameters will drop from
O(N)(where N is the number of years) toO(1)(bottlenecked only by the slowest single-year network request).🔬 Measurement:
npm run buildor profile the execution time ofgenerateStaticParamsin these files locally before and after the change.PR created automatically by Jules for task 7659829852147852428 started by @anyulled
Summary by CodeRabbit