Load the FAQ articles on demand instead of in every route's locale bundle - #1616
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
PR Summary by QodoLazy-load en-US FAQ translations via build-time locale split
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
Greptile SummaryThe PR removes English FAQ articles from the eagerly bundled locale and loads them only for FAQ-dependent surfaces.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/web/next.config.js | Registers the targeted English locale loader for both core and query-qualified FAQ imports. |
| apps/web/src/features/i18n/faq-split.js | Splits FAQ article headers and bodies from the eagerly loaded English locale while retaining core FAQ strings. |
| apps/web/src/features/i18n/faq.ts | Implements deduplicated on-demand FAQ loading, English fallback registration, and server resource extraction. |
| apps/web/src/features/i18n/use-faq-translations.ts | Exposes current-locale readiness and retries transient loading failures while preventing stale loads from marking another locale ready. |
| apps/web/src/features/i18n/faq-resources.tsx | Synchronously primes client-side English FAQ resources supplied by the FAQ server page. |
| apps/web/src/app/(staticPages)/faq/page.tsx | Preloads server FAQ translations and transfers English article resources for hydration and client-side search. |
| apps/web/src/specs/features/i18n/faq-split.spec.ts | Covers locale splitting, resource restoration, fallback loading, idempotent priming, and full-locale loading after partial registration. |
Sequence Diagram
sequenceDiagram
participant Surface as FAQ-dependent surface
participant Hook as useFaqTranslations
participant Loader as ensureFaqLoaded
participant Locale as i18next resources
Surface->>Hook: Request readiness for active language
Hook->>Loader: Load active-language FAQ resources
alt English
Loader->>Loader: Import en-US.json?faq
else Other locale
Loader->>Locale: Load whole locale bundle
Loader->>Loader: Load English fallback articles
end
Loader->>Locale: Merge translation resources
Loader-->>Hook: Load settled
Hook->>Locale: Probe current active language
Hook-->>Surface: Render when ready
Reviews (2): Last reviewed commit: "Load the FAQ articles on demand instead ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
edc0f94 to
4a2b50a
Compare
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| resolveId(this: any, id: string, importer: string | undefined) { | ||
| if (!id.endsWith('en-US.json?faq')) return null; | ||
| return this.resolve(id.replace('?faq', ''), importer, { skipSelf: true }); | ||
| } |
There was a problem hiding this comment.
2. New any in vitest plugin 📘 Rule violation ⚙ Maintainability
apps/web/vitest.config.mts introduces any types (this: any and as any) in new code, violating the ban on new any usage. This reduces type safety and can mask real typing issues in the test/build tooling layer.
Agent Prompt
## Issue description
New code in `vitest.config.mts` introduces `any` (`this: any` and `faqQuery as any`), which violates the rule disallowing new `any`/implicit-any usage in changed TypeScript.
## Issue Context
The file comment notes plugin typing mismatches due to dual Vite versions; however, the code can still avoid `any` by using `unknown` plus a narrow cast for the minimal `this.resolve(...)` shape, or by using a compatible Vite `Plugin`/`PluginContext` type without falling back to `any`.
## Fix Focus Areas
- apps/web/vitest.config.mts[25-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Taken: the plugin is now typed from vitest's own UserConfig['plugins'] element type, so this.resolve is typed by the plugin context and the as any on it is gone (the pre-existing stubStyles cast is untouched).
| .then(() => { | ||
| if (!cancelled) setReady(true); | ||
| }) |
There was a problem hiding this comment.
3. Stale load marks language ready 🐞 Bug ≡ Correctness
useFaqTranslations can set ready=true when an earlier locale load resolves even if i18n.language has since changed and the current language is still loading. This can cause consumers to render fallback/raw keys or stale-language FAQ content and may remain stuck if the later “current language” completion tries to set ready to true again and React skips the no-op update.
Agent Prompt
## Issue description
`useFaqTranslations` can incorrectly mark itself `ready` when an older locale’s FAQ resource load completes after i18next has already switched languages, so consumers may render/search FAQ strings for the new locale before its bundle is available (showing fallback English, raw keys, incorrect results, or stale-language content).
## Issue Context
The hook starts asynchronous loads per language in response to language changes, but it only cancels/guards on unmount and unconditionally sets `ready=true` on any completion without checking that the completion matches the active language (or the latest in-flight request). During rapid language switches, an earlier request can resolve later and flip readiness early; when the real current-language load finishes, calling `setReady(true)` again may be a no-op, potentially leaving the UI stuck until another render. Fix by associating each load with the language (or a monotonically increasing request generation) and only updating readiness when the resolved request still matches the active/latest language, recomputing readiness for the active language rather than unconditionally setting it to true.
## Fix Focus Areas
- apps/web/src/features/i18n/use-faq-translations.ts[15-32]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
This was fixed in the previous push: readiness is re-probed against i18n.language at the moment a load settles (setReady(isFaqLoaded(i18n.language))), so a load that started for an earlier language cannot mark the current one ready, and the language-change handler cancels a pending retry and starts a fresh load; a later completion for the current language sets ready from its own probe, so there is no stuck no-op update.
4a2b50a to
9e460e7
Compare
…ndle
en-US.json is the only eagerly bundled locale, and its FAQ articles
(static.faq.*-header / *-body: 262 keys, about 17 KB gzipped, a quarter
of the file) shipped in first-load JS on every route although only the
FAQ surfaces render them.
A webpack loader now splits that file at build time: the plain import
gets the locale without the articles and a ?faq import gets only them,
so the JSON on disk stays whole and Crowdin keeps one source file. The
other locales are already loaded on demand as whole files and need no
change. ensureFaqLoaded merges the English articles back into the
translation namespace (and loads them alongside any other language, since
English is the per-key fallback), so every existing
i18next.t("static.faq.…") call keeps working once it has resolved. The
FAQ and About pages await it on the server; the FAQ page hands the
English articles to its client components through <FaqResources> so they
hydrate with the strings the server rendered; the help-center search,
decks FAQ column and the two perks explainers render their FAQ strings
once useFaqTranslations reports them present. Only English is ever
primed from the server: the other locales belong to loadLocale as whole
files, whose guard now probes a core key so a partial bundle can never
pass for the full file. A vitest resolver maps the ?faq import to the
plain file in tests.
Closes #1598
Code Review by Qodo
1. useFaqTranslations missing effect deps
|
9e460e7 to
44a9b62
Compare
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| ensureFaqLoaded(i18n.language) | ||
| .then(() => { | ||
| if (!cancelled) setReady(true); |
There was a problem hiding this comment.
2. Stale load marks ready 🐞 Bug ≡ Correctness
useFaqTranslations sets ready=true when any in-flight ensureFaqLoaded call resolves without verifying i18next is still on the language that started the request. During overlapping language changes, an earlier load can mark readiness for a newer locale that is still loading, causing consumers to render/filter FAQ content using fallback or stale translations (including raw static.faq.* keys) and potentially preventing dependent effects from rerunning because ready is already true.
Agent Prompt
## Issue description
`useFaqTranslations` publishes readiness from an asynchronous FAQ load even if the active language changed after that load started. As a result, a completion for a request initiated under an earlier language can incorrectly set `ready=true` for a newer locale that is still loading, causing consumers to render fallback/stale FAQ translations (including raw `static.faq.*` keys) and potentially skipping recomputation paths that depend on `ready` because it is already `true`.
## Issue Context
`load()`/`ensureFaqLoaded` is triggered on each `languageChanged` event and FAQ resource loading is asynchronous per language. Capture the language associated with each initiated load and only update readiness when the promise resolves if that language still matches the currently active i18next language; additionally ensure that completion of the current language load triggers consumer updates (rendering and FAQ-search effects) even if an older request already completed.
## Fix Focus Areas
- apps/web/src/features/i18n/use-faq-translations.ts[15-32]
- apps/web/src/features/i18n/use-faq-translations.ts[17-22]
- apps/web/src/features/i18n/faq.ts[45-62]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Already fixed in a later push (this comment is on the first commit): readiness is re-probed against i18n.language when a load settles, so a load started for an earlier language cannot mark the current one ready, and a language change cancels any pending retry and starts a fresh load.
| .catch(() => { | ||
| /* the strings stay hidden; nothing else to do */ | ||
| }); |
There was a problem hiding this comment.
3. Failed load hides content 🐞 Bug ☼ Reliability
The hook suppresses every ensureFaqLoaded rejection and leaves ready false permanently until a language change or remount. A transient FAQ chunk or locale load failure therefore removes the explainer text and FAQ lists for the rest of the mounted session, with neither retry nor visible error, whereas these strings were previously available from the eager English fallback.
Agent Prompt
## Issue description
FAQ loading failures are silently converted into permanently hidden content. The hook neither retries nor exposes an error/fallback state, so transient chunk failures remove user-facing FAQ articles for the component's mounted lifetime.
## Issue Context
English is the per-key fallback and non-English loading already requests English alongside the active locale. Preserve usable fallback content where possible, and provide bounded retry or an explicit recoverable error state rather than swallowing the rejection indefinitely.
## Fix Focus Areas
- apps/web/src/features/i18n/use-faq-translations.ts[15-32]
- apps/web/src/features/i18n/faq.ts[45-62]
- apps/web/src/app/perks/points/_components/points-basic-info.tsx[30-35]
- apps/web/src/app/perks/promote-post/_components/promote-post-intro.tsx[19-23]
- apps/web/src/features/ecency-center/sections/center-faq.tsx[71-89]
- apps/web/src/app/decks/_components/columns/deck-faq-column.tsx[65-74]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Already fixed in a later push: a failed load is retried twice with a delay before the strings stay hidden. English fallback content cannot be shown instead, because the English articles are exactly what failed to load in that case; until they arrive the consumers render nothing rather than raw keys.
There was a problem hiding this comment.
Your organization has used all 50 credits included in the free plan this billing period. To keep receiving reviews, upgrade your plan.
1 participant
en-US.jsonis the only eagerly bundled locale, and its FAQ articles (static.faq.*-header/*-body: 262 keys, about 17 KB gzipped, a quarter of the file) shipped in first-load JS on every route although only the FAQ surfaces render them.A webpack loader (
features/i18n/faq-split.js) now splits that file at build time: the plain import gets the locale without the articles and a?faqimport gets only them, so the JSON on disk stays whole and Crowdin keeps one source file. The other locales are already loaded on demand as whole files and need no change.ensureFaqLoadedmerges the English articles back into the translation namespace (and loads them alongside any other language, since English is the per-key fallback), so every existingi18next.t("static.faq.…")call keeps working once it has resolved. The FAQ and About pages await it on the server; the FAQ page hands the English articles to its client components through<FaqResources>so they hydrate with the strings the server rendered; the help-center search, the decks FAQ column and the two perks explainers render their FAQ strings onceuseFaqTranslationsreports them present. Only English is ever primed from the server: the other locales belong toloadLocaleas whole files, and its guard now probes a core key so a partial bundle can never pass for the full file (this was the one real finding in review).Measured on the production build: the eager en-US chunk goes from 77 KB to 61 KB gzipped on every route, the articles become a 17 KB on-demand chunk that no route lists in first-load, and the feed route's first-load JS is 779 KB gzipped. Trade-off stated plainly:
/faqitself now carries the English articles once more inside its RSC payload (about 17 KB gzipped) so its client components can hydrate and search without a flash, which is a small loss on that one page against the saving everywhere else.Test plan
faq-split.spec.ts: the loader moves every article and nothing else, keeps the small FAQ keys, serves core vs?faqby resource query; on the real i18next instance the articles are reported missing, merged back, exposed for the client, primed idempotently, loaded alongside another language, and a partial bundle no longer stopsloadLocalefrom fetching the whole file. Full suite (334 files, 3,160 tests),tsc --noEmitandnext lintclean./faqserver-renders the article text, the category lists hydrate with real headers and client-side search over bodies works;/faq?lang=esrenders Spanish on the server and the client switch loads the whole Spanish locale (UI strings included);/aboutand/perks/pointsrender their FAQ text; the home page loads no FAQ chunk. The React Made waves form actions visible permanently #418 hydration warning seen on/faqand/perks/points, and the rawwhat-gift-button-meanskey on/faq, reproduce on the current develop build as well and are unrelated to this change.Closes #1598