Conversation
6c5179d to
48eb0f0
Compare
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthroughThis PR upgrades Next.js from 15.5.14 to 16.2.0, migrating ESLint configuration to flat config format, updating TypeScript compilation settings, adding "use client" directives across components, removing webpack customizations, and applying defensive null/undefined handling throughout the codebase. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ 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 |
48eb0f0 to
9bcf717
Compare
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
front_end/src/app/(main)/questions/actions.ts (1)
309-312: ReplacerevalidateTagwithupdateTagfor immediate consistency after deletion.After
removeRelatedArticle(articleId)completes, the cachedrelated-articlesdata should be immediately fresh, not stale-while-revalidate. While the component uses local state to provide instant UI feedback, usingupdateTag("related-articles")ensures backend consistency across all views and edge cases (multi-tab refresh, navigation, other components).Change:
revalidateTag("related-articles", "max");to:
updateTag("related-articles");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/actions.ts around lines 309 - 312, The code currently calls revalidateTag("related-articles", "max") after awaiting ServerPostsApi.removeRelatedArticle(articleId) in removeRelatedArticle; replace that call with updateTag("related-articles") so the cache for the "related-articles" tag is updated immediately across clients. Locate the removeRelatedArticle function and change the revalidateTag invocation to updateTag("related-articles") to ensure immediate consistency after ServerPostsApi.removeRelatedArticle completes.front_end/src/app/(main)/questions/components/numeric_question_input.tsx (1)
172-187: Minor inconsistency in null-handling pattern.The Date branch still uses
(min ? min : 0)/(max ? max : 0)(lines 179, 183) while the Numeric branch now uses?? 0. For Date questions this is fine since timestamps are always > 0, but consider aligning to?? 0for consistency with the rest of the PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/src/app/`(main)/questions/components/numeric_question_input.tsx around lines 172 - 187, The null-handling in the Date formatting inside current_errors.push uses (min ? min : 0) and (max ? max : 0); change those to use the nullish coalescing operator (min ?? 0) and (max ?? 0) to match the Numeric branch and existing PR style. Locate the call to t.rich("zeroPointError1", { zeroPoint: format(new Date(zeroPoint * 1000), ...), min: format(new Date((min ? min : 0) * 1000), ...), max: format(new Date((max ? max : 0) * 1000), ...) }) and replace the ternaries with ?? 0 for min and max so date formatting uses the same null-handling pattern.front_end/next.config.mjs (1)
31-45: Limit the SVG transform to non-foreignimports.These conditions currently rewrite every
*.svgimport, includingnode_modulesand some Next internals. Turbopack documentsforeignfor scoping loaders away from those modules; without it, a dependency import can be transformed into your app’s SVGR/asset behavior unexpectedly. I’d also match the?urlflag exactly instead of any query that happens to containurl. (nextjs.org)Safer Turbopack rule shape
turbopack: { rules: { "*.svg": [ { - condition: { query: /\burl\b/ }, + condition: { + all: [{ not: "foreign" }, { query: /[?&]url(?=&|$)/ }], + }, type: "asset", }, { - condition: { not: { query: /\burl\b/ } }, + condition: { + all: [{ not: "foreign" }, { not: { query: /[?&]url(?=&|$)/ } }], + }, loaders: ["@svgr/webpack"], as: "*.js", }, ], }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front_end/next.config.mjs` around lines 31 - 45, Update the "*.svg" turbopack rule so it doesn't apply to foreign (external/dependency/Next-internal) imports and only treats exact ?url queries as URL imports: add a condition excluding foreign resources (e.g., wrap each branch with a condition like { not: { resource: { origin: "foreign" } } } or equivalent turbopack "foreign" scoping) and replace the loose /\burl\b/ query tests with an exact match for the ?url flag (e.g., /\?url$/) in the two condition entries inside the turbopack.rules["*.svg"] rule so the asset/loader branches only target your app sources and exact ?url imports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front_end/next.config.mjs`:
- Line 1: The current generateBuildId uses crypto.randomUUID() causing a new
build ID on each build; change generateBuildId to return a stable identifier
sourced from CI/GIT (e.g., process.env.BUILD_ID, GIT_COMMIT_SHA, or a CI
deployment id) and fall back to that order rather than generating a UUID; also
set the Next.js deploymentId to the same stable value so the runtime skew
protection sees identical IDs; update any code that relies on
process.env.BUILD_ID to use that env var (or the git/CI fallback) and ensure
generateBuildId and deploymentId consistently return the same stable identifier
across replicas.
In
`@front_end/src/app/`(main)/questions/[id]/components/sidebar/sidebar_question_info.tsx:
- Around line 27-30: The pluralization logic is inverted: change the count
passed to the i18n call in sidebar_question_info.tsx so it reflects the actual
total number of authors rather than presence of coauthors; replace the current
count expression using postData.coauthors.length > 0 with a total authors value
(e.g., 1 + postData.coauthors.length) when calling t("authorWithCount", { count:
... }) so the ICU pattern in t() receives 1 for single author and 2+ for
multiple authors.
In `@front_end/src/components/search_input.tsx`:
- Line 1: SearchInput was marked a Client Component which creates an RSC
boundary that breaks because the Server Component question_picker is passing
inline function props (onChange, onErase) across the boundary; either convert
question_picker to a Client Component so it owns the interactive state and can
pass handlers directly to SearchInput (add "use client" and move the
onChange/onErase state/handlers into the question_picker component), or keep
question_picker as a Server Component and add a small Client wrapper (e.g.,
QuestionPickerClient or SearchInputWrapper) that lives inside question_picker,
owns the search state and implements/onChange and onErase handlers, and then
forwards those handlers as props to SearchInput so no server-to-client function
props are passed.
In `@front_end/src/types/translations.ts`:
- Line 1: The change making TranslationKey an unconstrained string removes
compile-time i18n key checking; either document this decision and add
runtime/test safeguards or restore stricter typing via next-intl AppConfig
augmentation. Update the repo by (1) adding a short comment in front of the
TranslationKey type explaining the intentional choice to keep it as string
(reference: TranslationKey) and why dynamic key patterns in files like
dataset.ts and additional_scores_table.tsx require it, (2) if leaving it as
string, add runtime validation or unit/integration tests to assert expected keys
exist before rendering (or point to the test file/location), and (3) if you
prefer compile-time safety, reintroduce the AppConfig augmentation approach in
the i18n types to narrow TranslationKey. Ensure the chosen approach is
documented in-code and covered by tests or augmentation to prevent missing
translation keys at runtime.
---
Nitpick comments:
In `@front_end/next.config.mjs`:
- Around line 31-45: Update the "*.svg" turbopack rule so it doesn't apply to
foreign (external/dependency/Next-internal) imports and only treats exact ?url
queries as URL imports: add a condition excluding foreign resources (e.g., wrap
each branch with a condition like { not: { resource: { origin: "foreign" } } }
or equivalent turbopack "foreign" scoping) and replace the loose /\burl\b/ query
tests with an exact match for the ?url flag (e.g., /\?url$/) in the two
condition entries inside the turbopack.rules["*.svg"] rule so the asset/loader
branches only target your app sources and exact ?url imports.
In `@front_end/src/app/`(main)/questions/actions.ts:
- Around line 309-312: The code currently calls
revalidateTag("related-articles", "max") after awaiting
ServerPostsApi.removeRelatedArticle(articleId) in removeRelatedArticle; replace
that call with updateTag("related-articles") so the cache for the
"related-articles" tag is updated immediately across clients. Locate the
removeRelatedArticle function and change the revalidateTag invocation to
updateTag("related-articles") to ensure immediate consistency after
ServerPostsApi.removeRelatedArticle completes.
In `@front_end/src/app/`(main)/questions/components/numeric_question_input.tsx:
- Around line 172-187: The null-handling in the Date formatting inside
current_errors.push uses (min ? min : 0) and (max ? max : 0); change those to
use the nullish coalescing operator (min ?? 0) and (max ?? 0) to match the
Numeric branch and existing PR style. Locate the call to
t.rich("zeroPointError1", { zeroPoint: format(new Date(zeroPoint * 1000), ...),
min: format(new Date((min ? min : 0) * 1000), ...), max: format(new Date((max ?
max : 0) * 1000), ...) }) and replace the ternaries with ?? 0 for min and max so
date formatting uses the same null-handling pattern.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 03625de1-b768-48d5-a735-1518960e4a87
⛔ Files ignored due to path filters (1)
front_end/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
front_end/.eslintignorefront_end/.eslintrc.jsonfront_end/eslint.config.mjsfront_end/next.config.mjsfront_end/package.jsonfront_end/src/app/(futureeval)/futureeval/components/benchmark/performance-over-time/collision-aware-labels.tsxfront_end/src/app/(main)/(leaderboards)/medals/components/medal_card.tsxfront_end/src/app/(main)/accounts/profile/[id]/medals/page.tsxfront_end/src/app/(main)/aggregation-explorer/components/question_metadata.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/sidebar_question_info.tsxfront_end/src/app/(main)/questions/actions.tsfront_end/src/app/(main)/questions/components/coherence_links/link_strength_component.tsxfront_end/src/app/(main)/questions/components/numeric_question_input.tsxfront_end/src/app/(main)/services/components/case_studies/case_study_card.tsxfront_end/src/components/forecast_maker/conditional_forecast_table.tsxfront_end/src/components/forecast_maker/continuous_group_accordion/accordion_open_button.tsxfront_end/src/components/forecast_maker/continuous_group_accordion/group_forecast_accordion_item.tsxfront_end/src/components/forecast_maker/forecast_expiration.tsxfront_end/src/components/forecast_maker/forecast_text_input.tsxfront_end/src/components/markdown_editor/__tests__/initialized_editor.test.tsxfront_end/src/components/onboarding/steps/step_1.tsxfront_end/src/components/onboarding/steps/step_2.tsxfront_end/src/components/popover_filter/combobox_filter.tsxfront_end/src/components/search_input.tsxfront_end/src/components/ui/button.tsxfront_end/src/components/ui/chip.tsxfront_end/src/components/ui/info_toggle_container.tsxfront_end/src/components/ui/inline_select.tsxfront_end/src/components/ui/listbox.tsxfront_end/src/components/ui/select.tsxfront_end/src/components/ui/switch.tsxfront_end/src/proxy.tsfront_end/src/sentry/options.tsfront_end/src/types/translations.tsfront_end/tsconfig.json
💤 Files with no reviewable changes (2)
- front_end/.eslintignore
- front_end/.eslintrc.json
front_end/src/app/(main)/questions/[id]/components/sidebar/sidebar_question_info.tsx
Show resolved
Hide resolved
…l to v4 with full Turbopack migration
9bcf717 to
0da490b
Compare
cemreinanc
left a comment
There was a problem hiding this comment.
spotted 2 regressions and commented about them.
also we need to test about reverting this change: #2369 and discussion about it on vercel/next.js#82651
Maybe its solved in upstream turbopack but I cant find a solid result about it so we need to explore.
| ], | ||
| }, | ||
| }, | ||
| images: { |
There was a problem hiding this comment.
this is from migration doc:
If you specify a quality prop not included in the image.qualities array, the quality will be coerced to the closest value in images.qualities.
so many quality 100 images in our codebase will be rendered at 75.
add qualities: [75, 100] to prevent not configured quality errors or remove quality=100 configs from these images if that's not necessary.
| import { MessageKeys } from "next-intl"; | ||
|
|
||
| export type TranslationKey = MessageKeys<IntlMessages, keyof IntlMessages>; | ||
| export type TranslationKey = string; |
There was a problem hiding this comment.
TranslationKey type regression — Type safety silently broken
Root cause
next-intl v4 changed its type augmentation mechanism. The project is still using the v3 pattern which v4 no longer reads:
v3 (current in global.d.ts):
declare global {
interface IntlMessages extends Messages {} // v4 ignores this
}v4 (what the library now expects):
declare module 'next-intl' {
interface AppConfig {
Messages: typeof en;
}
}In v4, AppConfig in use-intl/dist/types/core/AppConfig.d.ts resolves Messages like this:
export type Messages = AppConfig extends {
Messages: infer AppMessages;
} ? AppMessages : Record<string, any>; // <-- falls back to untypedSince AppConfig is never augmented in the project, all translation keys fall back to Record<string, any> — meaning t("anyTypo") compiles without error. The TranslationKey = string change was likely a consequence of the old MessageKeys<IntlMessages, ...> silently breaking.
Impact
useTranslations()→t()accepts any string with no autocomplete or compile-time validationTranslationKeyisstring— no typo detectionkeyof IntlMessagesreferences inquestion_metadata.tsxandlink_strength_component.tsxwere also replaced withstring
Fix
Update global.d.ts to use the v4 AppConfig pattern:
import { RowData } from "@tanstack/table-core";
import en from "./messages/en.json";
import "@tanstack/react-table";
declare module 'next-intl' {
interface AppConfig {
Messages: typeof en;
}
}
declare module "@tanstack/table-core" {
interface ColumnMeta<TData extends RowData, TValue> {
className: string;
}
}Once that's done, MessageKeys is still exported from use-intl/core in v4, so you can restore typed TranslationKey:
import { MessageKeys } from "next-intl";
export type TranslationKey = MessageKeys<Messages, keyof Messages>;Or just use keyof IntlMessages pattern again once AppConfig makes the types flow through.
Closes #4547
This PR upgrades the frontend from Next.js 15 to 16.2 with full Turbopack adoption, along with
companion upgrades for React, Sentry, next-intl, ESLint, and Storybook.
What Changed and Why
Turbopack Migration (
next.config.mjs)I removed entire webpack callback (SVG handling + buildId injection + eslint config).
Added
turbopack.ruleswith condition-based SVG handling:*.svg?urlimports –type: "asset"(returns URL string)*.svgimports –@svgr/webpackloader (returns React component)This uses Turbopack 16.2's new
condition.queryfeature to replicate the webpackresourceQuerybehavior.Replaced
webpack.DefinePluginwithgenerateBuildId()+env.BUILD_ID. A UUID is generated at config evaluation time, used both as Next.js build ID and injected as an environment variable for the version checker.Removed
eslint: { ignoreDuringBuilds: true }– this config option was removed in Next.js 16 (along with thenext lintcommand).Middleware to Proxy (
middleware.tstoproxy.ts)Next.js 16 renames the middleware convention to "proxy" to clarify its role as a network boundary layer. The file was renamed and the exported function changed from
middlewaretoproxy. Behavior is identical.ESLint Flat Config (
.eslintrc.jsontoeslint.config.mjs)next lintwas removed in Next.js 16 –lint:jsscript now callseslint .directlyeslint-config-nextv16 exports native flat config arrays – imported directly instead of throughFlatCompat(which caused circular reference errors with the React plugin)FlatCompatis still used forprettierandstorybookplugins that don't yet have native flat config exportsreact-hooks/set-state-in-effect,react-hooks/purity, etc.) are set towarnfor incremental adoptionSentry v10 (
@sentry/nextjs)Required upgrade because Sentry v9's
withSentryConfiginjectedexperimental.turbo(the old config key), causing Next.js 16 to show "unrecognized key" warnings and breaking the next-intl plugin.next-intl v4
Required upgrade because next-intl v3's plugin checked
process.env.TURBOPACKand injectedexperimental.turbo.resolveAlias– the deprecated config key that Next.js 16 doesn't understand. This caused the "Couldn't find next-intl config file" build error.Performance
next devstartupnext buildcompileSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores