feat: optimize generateStaticParams data fetching performance#262
feat: optimize generateStaticParams data fetching performance#262anyulled wants to merge 2 commits into
Conversation
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
11 rules 1. getJobOffersForEdition() errors unhandled
|
|
Warning Review limit reached
More reviews will be available in 49 minutes and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughThis PR documents and applies a concurrency optimization pattern across Next.js dynamic routes. A new guidance entry describes using ChangesStatic Parameters Concurrency Optimization
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 unit tests (beta)
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 |
Review Summary by QodoOptimize
WalkthroughsDescription• Parallelize static param fetching • Preserve per-year error isolation • Flatten batched parameter arrays • Update project notes with guidance Diagramflowchart LR
Y["Archived or available years"] --> P["Promise.all over years"]
P --> J["Job offers params"]
P --> S["Speaker params"]
P --> T["Talk params"]
P --> G["Tag params"]
J --> F["Flatten results"]
S --> F
T --> F
G --> F
F --> R["Return static params"]
File Changes1. .jules/bolt.md
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the generateStaticParams function across several pages (job-offers, speakers, tags, and talks) by refactoring sequential loops into parallelized data fetching using Promise.all and flattening the results. It also documents this optimization pattern in .jules/bolt.md. The reviewer recommends using strict mode (e.g., { strict: true }) for the data fetching functions (getSpeakers and getTalks) and removing the try/catch blocks. This ensures that any data fetching failures during the build process correctly trigger a build failure, preventing the deployment of incomplete pages. The documentation in .jules/bolt.md should also be updated to reflect this strict error-handling approach.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const yearParamsArrays = await Promise.all( | ||
| 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.
According to the general rules, data fetching functions used during the build process (such as generateStaticParams) should use strict mode to throw an error on failure. This ensures that any data fetching problems cause the build to fail, preventing the deployment of incomplete pages (which would otherwise result in 404 errors since dynamicParams = false).
Please call getSpeakers(year, { strict: true }) and remove the try/catch block so that failures are not silently ignored.
const yearParamsArrays = await Promise.all(
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 (e.g., for sitemap generation) should include a 'strict' mode that throws an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
| const yearParamsArrays = await Promise.all( | ||
| 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.
According to the general rules, data fetching functions used during the build process should use strict mode to throw an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
Please call getTalks(year, { strict: true }) and remove the try/catch block so that failures are not silently ignored.
const yearParamsArrays = await Promise.all(
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 (e.g., for sitemap generation) should include a 'strict' mode that throws an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
| const yearParamsArrays = await Promise.all( | ||
| 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.
According to the general rules, data fetching functions used during the build process should use strict mode to throw an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
Please call getTalks(year, { strict: true }) and remove the try/catch block so that failures are not silently ignored.
const yearParamsArrays = await Promise.all(
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 (e.g., for sitemap generation) should include a 'strict' mode that throws an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
| **Learning:** In Next.js applications, sequential data fetching inside `generateStaticParams` (e.g., using `for...of` loops over an array of years) blocks the build process. Next.js statically builds pages in parallel, but fetching the paths shouldn't be a bottleneck. | ||
| **Action:** Always map over the independent data (like years) and use `Promise.all` to fetch the parameters in parallel. Retain the per-item `try/catch` inside the map to gracefully return empty arrays for failed items without failing the entire batch, and finish with `.flat()` to combine the parameter arrays. |
There was a problem hiding this comment.
The documented action suggests retaining a per-item try/catch to gracefully return empty arrays for failed items. However, this conflicts with the general rule that data fetching functions used during the build process should use strict mode to throw an error on failure, ensuring that data fetching problems cause the build to fail and preventing the deployment of incomplete pages.
Please update the action to specify using strict mode and letting the build fail on fetch errors.
References
- Data fetching functions used during the build process (e.g., for sitemap generation) should include a 'strict' mode that throws an error on failure. This ensures that data fetching problems cause the build to fail, preventing the deployment of incomplete pages.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@app/`[year]/job-offers/[companyName]/page.tsx:
- Around line 24-32: The per-year fetch inside years.map is missing the
try/catch present in other refactors; wrap the async callback passed to
years.map in a try { const companies = await getJobOffersForEdition(year);
return companies.map(...) } catch (err) { console.warn(`Failed to fetch job
offers for year ${year}:`, err); return []; } so that
Promise.all(yearParamsArrays) cannot be rejected by an unexpected throw;
reference the years.map callback, getJobOffersForEdition, and yearParamsArrays
when locating where to add the try/catch and the console.warn fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4bdb052-3447-4d97-9380-3a6f2499c150
📒 Files selected for processing (5)
.jules/bolt.mdapp/[year]/job-offers/[companyName]/page.tsxapp/[year]/speakers/[speakerId]/page.tsxapp/[year]/tags/[tag]/page.tsxapp/[year]/talks/[talkId]/page.tsx
| const yearParamsArrays = await Promise.all( | ||
| years.map(async (year) => { | ||
| const companies = await getJobOffersForEdition(year); | ||
| return companies.map((company) => ({ | ||
| year, | ||
| companyName: company.id, | ||
| }); | ||
| } | ||
| } | ||
| })); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add per-year error handling for consistency with documented pattern.
This implementation is missing the try/catch block that is present in the other three parallel refactors (speakers, tags, talks) and explicitly documented in the guidance (.jules/bolt.md lines 7-8: "Retain the per-item try/catch inside the map to gracefully return empty arrays for failed items").
While getJobOffersForEdition returns [] on internal errors, wrapping each year's fetch in try/catch ensures any unexpected throw won't reject the entire Promise.all, and provides explicit error visibility via console.warn as the other implementations do.
♻️ Proposed fix to add consistent error handling
const yearParamsArrays = await Promise.all(
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 [];
+ }
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const yearParamsArrays = await Promise.all( | |
| years.map(async (year) => { | |
| const companies = await getJobOffersForEdition(year); | |
| return companies.map((company) => ({ | |
| year, | |
| companyName: company.id, | |
| }); | |
| } | |
| } | |
| })); | |
| }) | |
| ); | |
| const yearParamsArrays = await Promise.all( | |
| 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 []; | |
| } | |
| }) | |
| ); |
🤖 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 24 - 32, The
per-year fetch inside years.map is missing the try/catch present in other
refactors; wrap the async callback passed to years.map in a try { const
companies = await getJobOffersForEdition(year); return companies.map(...) }
catch (err) { console.warn(`Failed to fetch job offers for year ${year}:`, err);
return []; } so that Promise.all(yearParamsArrays) cannot be rejected by an
unexpected throw; reference the years.map callback, getJobOffersForEdition, and
yearParamsArrays when locating where to add the try/catch and the console.warn
fallback.
Source: Coding guidelines
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const yearParamsArrays = await Promise.all( | ||
| years.map(async (year) => { | ||
| const companies = await getJobOffersForEdition(year); | ||
| return companies.map((company) => ({ | ||
| year, | ||
| companyName: company.id, | ||
| }); | ||
| } | ||
| } | ||
| })); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
1. getjoboffersforedition() errors unhandled 📘 Rule violation ≡ Correctness
generateStaticParams() runs concurrent per-year data fetching with Promise.all(...) and awaits getJobOffersForEdition(year) without any try/catch or explicit .catch(...), so a single rejection will fail the entire batch and bubble up as an unhandled error. This violates the requirement to handle promise rejections explicitly and can unexpectedly break static path generation/builds rather than isolating per-year failures like other routes in this PR.
Agent Prompt
## Issue description
`generateStaticParams()` in the job-offers company page performs concurrent per-year fetching via `Promise.all`, but it does not explicitly handle promise rejections (no per-year `try/catch`, no `.catch(...)`, and no outer guard). Because `Promise.all` is fail-fast, any rejection from `getJobOffersForEdition(year)` will reject the entire batch and can fail static params generation/build.
## Issue Context
PR Compliance ID 95937 requires every awaited promise to be handled with `try/catch` or an explicit `.catch(...)`. Other `generateStaticParams()` implementations updated in this PR isolate per-year failures by wrapping each year’s fetch in `try/catch` and returning `[]` on failure (often logging), so that one bad year does not prevent other years from producing params.
## Fix Focus Areas
- app/[year]/job-offers/[companyName]/page.tsx[21-35]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 participant
💡 What
Refactored the
generateStaticParamsmethods inapp/[year]/job-offers/[companyName]/page.tsx,app/[year]/speakers/[speakerId]/page.tsx,app/[year]/talks/[talkId]/page.tsx, andapp/[year]/tags/[tag]/page.tsxto use parallel execution viaPromise.allinstead of sequentialfor...ofloops.🎯 Why
Next.js static parameter generation blocks the build process. Sequentially awaiting data for every archived/available year creates a significant bottleneck.
📊 Impact
Dramatically reduces the time required to fetch data during the Next.js static build phase by running the data fetching concurrently for all years. The build is noticeably faster.
🔬 Measurement
Measured the time to complete
generateStaticParamsusingconsole.timeandconsole.timeEndbefore and after the change using an isolated script, showing an immediate drop from sequential execution time to concurrent execution time. Also verified thatnpm run buildcompletes quickly and correctly.PR created automatically by Jules for task 9265315359211558488 started by @anyulled
Summary by CodeRabbit