refactor: add TRPC, simplify folder structure and add basic error request handling#15
refactor: add TRPC, simplify folder structure and add basic error request handling#15mattiacerutti merged 6 commits intomainfrom
Conversation
WalkthroughAdds TRPC (server + client) and a TRPC React provider, replaces two REST snippet endpoints with TRPC procedures, introduces a useGameSnippets hook and related type reorganizations, removes in-repo rate limiter and client fetch adapter, and updates many import paths to a feature-scoped shared types layout. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant useGameSnippets
participant TRPC as TRPC Query
participant Server
participant DB as Database
Client->>useGameSnippets: mount / activateLanguage / changeSnippet
activate useGameSnippets
useGameSnippets->>TRPC: query snippet.languages
activate TRPC
TRPC->>Server: snippet.languages
Server->>DB: getLanguages()
DB-->>Server: languages
Server-->>TRPC: languages
TRPC-->>useGameSnippets: availableLanguages
deactivate TRPC
useGameSnippets->>TRPC: query snippet.random(languageId)
activate TRPC
TRPC->>Server: snippet.random
Server->>DB: getRandomSnippets()
DB-->>Server: rawSnippets
Server-->>TRPC: rawSnippets
TRPC-->>useGameSnippets: rawSnippets
deactivate TRPC
useGameSnippets->>useGameSnippets: buildClientSnippets(rawSnippets)
useGameSnippets-->>Client: activeSnippet / isNextButtonLocked / error
deactivate useGameSnippets
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/components/providers/trpc-provider.tsx (1)
26-50: Consider adding an error boundary.The TRPC provider should be wrapped with an error boundary to gracefully handle query/mutation failures and prevent the entire app from crashing.
Consider adding an error boundary component around the provider or within it:
import {ErrorBoundary} from 'react-error-boundary'; export function TRPCReactProvider({children}: {children: ReactNode}) { const [queryClient] = useState(() => createQueryClient()); const [trpcClient] = useState(() => api.createClient({ links: [ httpBatchLink({ url: "/api/trpc", headers() { return { "x-internal-api": "true", }; }, }), ], }) ); return ( <ErrorBoundary fallback={<div>Something went wrong. Please refresh the page.</div>}> <QueryClientProvider client={queryClient}> <api.Provider client={trpcClient} queryClient={queryClient}> {children} </api.Provider> </QueryClientProvider> </ErrorBoundary> ); }src/app/layout.tsx (1)
15-17: Consider adding React Query DevTools for development.Adding React Query DevTools can significantly improve the debugging experience during development.
Install and add the DevTools (only in development):
npm install @tanstack/react-query-devtoolsThen update the provider:
import { ReactQueryDevtools } from '@tanstack/react-query-devtools'; // Inside TRPCReactProvider's return: <QueryClientProvider client={queryClient}> <api.Provider client={trpcClient} queryClient={queryClient}> {children} {process.env.NODE_ENV === 'development' && <ReactQueryDevtools initialIsOpen={false} />} </api.Provider> </QueryClientProvider>src/server/trpc/routers/snippet.ts (1)
11-26: Re‑evaluaterandomas a mutation and parity with the old REST route
- This endpoint is read‑only; consider making it a
queryinstead of amutationto better match tRPC semantics (and to simplify client usage viauseQuery) unless you explicitly want to avoid caching for randomness.- The legacy REST handler for
/api/snippets/randomappliedrateLimit(request)and wrappedgetRandomSnippetsin atry/catchto return a generic 500 error. Here, there’s no equivalent rate‑limit or error shaping; any thrownErrorwill surface as an internal tRPC error with its message. If you still need throttling and generic error messages, you may want a small middleware or localtry/catchthat throws aTRPCErrorwith a generic message instead.src/features/game/hooks/useGameSnippets.ts (2)
26-30: Expose a consistent error object instead of mixing boolean and error
languagesErrorcomes fromisError(boolean), whilerandomErroris an actual error object. UsinglanguagesError || randomErrormeanserrorcan be eithertrueor an error instance, which is awkward to consume.You can keep the shape consistent by destructuring
errorfrom the query instead:- const {data: availableLanguages, isError: languagesError} = api.snippet.languages.useQuery(); + const {data: availableLanguages, error: languagesError} = api.snippet.languages.useQuery(); const {error: randomError, ...fetchRandomSnippets} = api.snippet.random.useMutation(); - const error = languagesError || randomError; + const error = languagesError ?? randomError;This way
erroris always an error (ornull/undefined), and the caller doesn’t have to special‑case booleans.
31-40: Consider guarding and surfacing failures in background snippet refills
fetchSnippetsForLanguageandrefillSnippetQueuerely onmutateAsync, which rejects on network/server errors. InrefillSnippetQueue, those rejections will propagate to the fire‑and‑forget promise stored inbackgroundFetchPromise, potentially causing unhandled rejections and leavingbackgroundFetchPromise.currentstuck in a rejected state.You can make this more robust by handling errors and always clearing the ref:
- const refillSnippetQueue = async (language: ILanguage) => { - const snippets = await fetchSnippetsForLanguage(language); - addSnippetsToQueue(snippets); - }; + const refillSnippetQueue = async (language: ILanguage) => { + const snippets = await fetchSnippetsForLanguage(language); + addSnippetsToQueue(snippets); + }; + + const startBackgroundRefill = (language: ILanguage) => { + backgroundFetchPromise.current = refillSnippetQueue(language) + .catch((error) => { + // optional: log or hook into some error reporting + console.error("Background snippet refill failed", error); + }) + .finally(() => { + backgroundFetchPromise.current = null; + }); + };Then use
startBackgroundRefill(language)inchangeSnippetinstead of assigning the raw promise. This avoids unhandled rejections and ensures the ref is reset once the background work finishes.src/features/snippets/services/get-random-snippets.client.ts (1)
1-16: Snippet mapping helper looks good; only minor readability nit (optional).The client/server snippet type separation and
buildClientSnippetscomposition overbuildClientSnippetare sound and type-safe. If you want, you could spread themap(...).filter(...)pipeline over multiple lines for readability, but it’s not required.src/app/game/page.tsx (2)
61-72: Async language activation isn’t awaited/caught; consider explicit loading and error handling.Both
handleChangeLanguageandactivateLanguageEventcall the asyncactivateLanguagewithoutawaitor error handling. IfactivateLanguage(TRPC mutation + initialization) rejects, this can surface as an unhandled promise rejection while you separately rely on the hook’serrorto render the error UI.You might want to:
- Set
GameStatus.LOADINGwhen changing language, to match the snippet-change UX.awaitactivateLanguagein user-initiated flows (and optionally in the effect wrapper) and handle errors explicitly (even if just with atry/catchthat lets the hook’serrordrive the UI), to avoid unhandled rejections.For example:
-const handleChangeLanguage = async (selectedLanguage: ILanguage) => { - resetStopwatch(); - activateLanguage(selectedLanguage); - track("LANGUAGE_CHANGE", {language: selectedLanguage.name}); -}; +const handleChangeLanguage = async (selectedLanguage: ILanguage) => { + resetStopwatch(); + setStatus(GameStatus.LOADING); + try { + await activateLanguage(selectedLanguage); + track("LANGUAGE_CHANGE", {language: selectedLanguage.name}); + } catch { + // error state is already propagated via useGameSnippets().error + } +};and similarly wrapping
activateLanguageEventin atry/catchwithawaitif you want initial-load errors to be fully controlled rather than bubbling as unhandled rejections.Also applies to: 75-93
94-111: Guard against empty snippet list when reparsing on autoclosing changes.On autoclosing-mode changes you reparse
[activeSnippet, ...gameState.getSnippetQueue()], filter out anynullresults frombuildClientSnippet, and then callinitialize(activeLanguage, reparsedSnippets). If all reparses fail (even if unlikely),reparsedSnippetswill be empty andinitializewill throw (Cannot initialize game with empty snippets), taking down the effect.Consider short‑circuiting when the filtered list is empty, or handling that case more gracefully, e.g.:
-const reparsedSnippets = snippetsToReparse - .map((snippet) => buildClientSnippet(snippet.rawSnippet, autoClosingMode !== AutoClosingMode.DISABLED)) - .filter((snippet): snippet is IClientSnippet => snippet !== null); - -initialize(activeLanguage, reparsedSnippets); +const reparsedSnippets = snippetsToReparse + .map((snippet) => buildClientSnippet(snippet.rawSnippet, autoClosingMode !== AutoClosingMode.DISABLED)) + .filter((snippet): snippet is IClientSnippet => snippet !== null); + +if (reparsedSnippets.length === 0) { + // Optionally set an error state or just restore previous status instead of throwing. + setStatus(GameStatus.FINISHED); + return; +} + +initialize(activeLanguage, reparsedSnippets);Adjust the fallback behavior to whatever UX you prefer, but avoiding the throw will keep the app more resilient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
package.json(2 hunks)prisma/seed/files.ts(1 hunks)prisma/seed/languages.ts(1 hunks)src/app/api/snippets/languages/route.ts(0 hunks)src/app/api/snippets/random/route.ts(0 hunks)src/app/api/trpc/[trpc]/route.ts(1 hunks)src/app/game/layout.tsx(1 hunks)src/app/game/page.tsx(2 hunks)src/app/layout.tsx(2 hunks)src/components/providers/trpc-provider.tsx(1 hunks)src/core/config/env.ts(0 hunks)src/core/middleware/rate-limiter.ts(0 hunks)src/features/game/components/game-view.tsx(1 hunks)src/features/game/components/typing-area/character.tsx(1 hunks)src/features/game/components/typing-area/index.tsx(1 hunks)src/features/game/hooks/useGameSnippets.ts(1 hunks)src/features/game/hooks/useTyping.ts(1 hunks)src/features/game/logic/game-logic.ts(1 hunks)src/features/game/logic/typing/add-character.ts(1 hunks)src/features/game/logic/typing/delete-character.ts(1 hunks)src/features/game/logic/typing/delete-line.ts(1 hunks)src/features/game/logic/typing/delete-word.ts(1 hunks)src/features/game/logic/typing/shared.ts(1 hunks)src/features/game/state/game-store.ts(1 hunks)src/features/game/utils/character.ts(1 hunks)src/features/game/utils/typing-metrics.ts(1 hunks)src/features/settings/stores/settings-store.ts(1 hunks)src/features/shared/types/snippet.ts(1 hunks)src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts(0 hunks)src/features/snippets/infrastructure/repositories/language.repository.server.ts(1 hunks)src/features/snippets/infrastructure/repositories/snippet.repository.server.ts(1 hunks)src/features/snippets/logic/parsing/snippet-parse.client.ts(1 hunks)src/features/snippets/services/get-random-snippets.client.ts(2 hunks)src/features/snippets/services/get-random-snippets.server.ts(1 hunks)src/proxy.ts(1 hunks)src/server/trpc/context.ts(1 hunks)src/server/trpc/routers/index.ts(1 hunks)src/server/trpc/routers/snippet.ts(1 hunks)src/server/trpc/trpc.ts(1 hunks)src/shared/types/snippet.ts(0 hunks)
💤 Files with no reviewable changes (6)
- src/core/middleware/rate-limiter.ts
- src/shared/types/snippet.ts
- src/app/api/snippets/random/route.ts
- src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts
- src/core/config/env.ts
- src/app/api/snippets/languages/route.ts
🧰 Additional context used
🧬 Code graph analysis (24)
src/features/snippets/infrastructure/repositories/snippet.repository.server.ts (1)
src/shared/db/prisma.ts (1)
base(8-14)
src/components/providers/trpc-provider.tsx (1)
src/server/trpc/routers/index.ts (1)
AppRouter(8-8)
src/features/game/logic/typing/add-character.ts (1)
src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/server/trpc/routers/index.ts (2)
src/server/trpc/trpc.ts (1)
createTRPCRouter(6-6)src/server/trpc/routers/snippet.ts (1)
snippetRouter(7-27)
src/server/trpc/trpc.ts (1)
src/server/trpc/context.ts (1)
TRPCContext(13-13)
src/features/game/components/game-view.tsx (1)
src/shared/types/language.ts (1)
ILanguage(1-6)
src/app/layout.tsx (2)
src/components/providers/trpc-provider.tsx (1)
TRPCReactProvider(26-50)src/shared/components/theme-provider.tsx (1)
ThemeProvider(5-11)
src/app/api/trpc/[trpc]/route.ts (2)
src/server/trpc/routers/index.ts (1)
appRouter(4-6)src/server/trpc/context.ts (1)
createTRPCContext(7-11)
prisma/seed/files.ts (1)
src/shared/types/language.ts (1)
ILanguage(1-6)
src/features/snippets/logic/parsing/snippet-parse.client.ts (1)
src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/features/snippets/infrastructure/repositories/language.repository.server.ts (1)
src/shared/db/prisma.ts (1)
base(8-14)
src/features/game/logic/typing/delete-line.ts (1)
src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/features/game/components/typing-area/index.tsx (1)
src/shared/types/character.ts (5)
CharacterTypes(13-18)INormalCharacter(25-27)IBaseCharacter(20-23)IWhitespaceCharacter(35-38)WhitespaceTypes(7-11)
src/features/game/hooks/useGameSnippets.ts (5)
src/features/game/state/game-store.ts (1)
useGameStore(42-150)src/components/providers/trpc-provider.tsx (1)
api(9-9)src/features/shared/types/language.ts (1)
ILanguage(1-6)src/features/snippets/services/get-random-snippets.client.ts (1)
buildClientSnippets(14-16)src/features/game/config/game.ts (1)
REFRESH_BUTTON_MIN_DELAY(10-10)
prisma/seed/languages.ts (1)
src/shared/types/language.ts (1)
ILanguage(1-6)
src/features/game/hooks/useTyping.ts (1)
src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/features/snippets/services/get-random-snippets.server.ts (1)
src/shared/types/snippet.server.ts (1)
ISnippet(1-4)
src/server/trpc/routers/snippet.ts (5)
src/server/trpc/trpc.ts (2)
createTRPCRouter(6-6)publicProcedure(7-7)src/features/snippets/infrastructure/repositories/language.repository.server.ts (2)
getLanguages(14-25)doesLanguageExist(4-12)src/features/snippets/services/get-random-snippets.server.ts (1)
getRandomSnippets(7-35)src/app/api/snippets/random/route.ts (1)
GET(6-32)src/features/snippets/infrastructure/adapters/snippet-fetch.client.ts (1)
fetchRandomSnippets(8-20)
src/features/game/logic/game-logic.ts (1)
src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/features/game/components/typing-area/character.tsx (1)
src/shared/types/character.ts (3)
IBaseCharacter(20-23)INormalCharacter(25-27)CharacterTypes(13-18)
src/app/game/page.tsx (4)
src/features/game/state/game-store.ts (1)
useGameStore(42-150)src/features/game/hooks/useGameSnippets.ts (1)
useGameSnippets(12-86)src/features/shared/types/language.ts (1)
ILanguage(1-6)src/features/snippets/services/get-random-snippets.client.ts (1)
buildClientSnippet(5-12)
src/app/game/layout.tsx (1)
src/shared/components/theme-toggle.tsx (6)
ThemeToggle(7-49)isMounted(19-22)frame(12-15)setIsMounted(13-13)cancelAnimationFrame(14-14)e(30-30)
src/features/shared/types/snippet.ts (3)
src/features/shared/types/character.ts (1)
ICharacter(45-45)src/features/shared/types/snippet.server.ts (1)
ISnippet(1-4)src/shared/types/snippet.ts (1)
ISnippet(6-9)
src/features/settings/stores/settings-store.ts (1)
src/shared/types/language.ts (1)
ILanguage(1-6)
🔇 Additional comments (40)
src/features/game/utils/typing-metrics.ts (1)
1-2: Import refactoring verified successfully.All imports across the codebase have been consistently updated from
@/shared/types/to@/features/shared/types/. Zero old import paths remain, confirming complete and uniform migration. The file under review is correctly updated.prisma/seed/files.ts (1)
1-1: LGTM! Consistent import path refactor.The import path update from
@/shared/types/languageto@/features/shared/types/languagealigns with the folder structure reorganization across the codebase.src/features/game/components/game-view.tsx (1)
4-4: LGTM! Consistent type import path update.The import path update maintains type consistency with the folder restructuring.
prisma/seed/languages.ts (1)
1-1: LGTM! Consistent import path update.src/features/settings/stores/settings-store.ts (1)
1-1: LGTM! Type import path aligned with restructuring.src/components/providers/trpc-provider.tsx (3)
11-24: LGTM! Solid retry configuration with exponential backoff.The retry logic implements exponential backoff (1s → 2s → 4s → capped at 10s) for both queries and mutations, which is appropriate for handling transient failures.
27-28: LGTM! Correct memoization pattern for React 19.Using
useStatewith initializer functions to memoizeQueryClientand TRPC client instances is the correct pattern for React 19, preventing unnecessary re-creation on re-renders.
33-37: Server-side validation confirmed—no action required.The header is properly validated in
src/proxy.ts(lines 5–10): the proxy function checks that the"x-internal-api"header equals"true"and returns a 403 Forbidden response if absent or invalid. This provides the required security check.src/app/game/layout.tsx (1)
4-4: LGTM! Import path aligned with component reorganization.src/app/layout.tsx (2)
15-17: LGTM! Correct provider nesting establishes TRPC context.The provider hierarchy (
TRPCReactProvider>ThemeProvider>children) is correct, ensuring TRPC and React Query contexts are available throughout the app.
3-4: LGTM! Import paths aligned with provider reorganization.package.json (2)
53-53: The review comment is incorrect—zod v4.1.12 is a valid, stable release.The npm registry confirms that
4.1.12is the current stable version of zod, not a non-existent version. The package.json dependency"zod": "^4.1.12"is correct and valid. There are also pre-release versions of 4.2.0 available as canary builds (dated November 2025). The original review was based on outdated information.Likely an incorrect or invalid review comment.
22-25: All package versions are secure.Verification found a HIGH severity tRPC WebSocket DoS vulnerability affecting versions < 11.1.1, but the PR uses version 11.7.1 for all tRPC packages, which is patched and safe. No vulnerabilities detected for React Query or other tRPC dependencies.
src/features/snippets/services/get-random-snippets.server.ts (1)
1-1: LGTM! Clean import path refactor.The import path update aligns with the folder structure simplification objective.
src/features/game/hooks/useTyping.ts (1)
8-8: LGTM! Import path updated correctly.The type import path has been successfully migrated to the feature-scoped location.
src/features/game/components/typing-area/character.tsx (1)
1-1: LGTM! Character type imports updated.The import path refactor is consistent with the folder structure reorganization.
src/features/game/logic/game-logic.ts (1)
1-2: LGTM! Multiple type imports migrated successfully.Both character and snippet type imports have been updated to the feature-scoped paths, maintaining type safety while supporting the folder structure refactor.
src/features/game/components/typing-area/index.tsx (1)
3-3: LGTM! Character type imports relocated.The import path update maintains functionality while aligning with the new feature-scoped structure.
src/features/game/logic/typing/delete-character.ts (1)
1-2: LGTM! Type imports updated consistently.Both character and snippet type imports have been successfully migrated to feature-scoped paths without affecting the deletion logic.
src/features/game/logic/typing/shared.ts (1)
2-3: LGTM! Shared typing utilities updated.Import paths successfully migrated while preserving all helper function implementations.
src/features/game/logic/typing/delete-line.ts (1)
3-3: Migration verification complete—no issues found.Verification confirms:
- ✓ No old
@/shared/types/snippetor@/shared/types/characterimports remain- ✓ New
@/features/shared/typespaths are in use across 18+ files- ✓ File
src/features/game/logic/typing/delete-line.tsline 3 contains the correct import:import {IParsedSnippet} from "@/features/shared/types/snippet";The import path refactor is complete and successful.
src/features/snippets/logic/parsing/snippet-parse.client.ts (1)
1-4: LGTM! Clean import path refactor.The import paths have been successfully updated to the new feature-scoped structure. This aligns with the PR's objective to reorganize shared types.
src/features/game/state/game-store.ts (1)
2-3: LGTM! Import paths correctly updated.The type imports have been successfully migrated to the feature-scoped module structure.
src/features/game/utils/character.ts (1)
1-2: LGTM! Import paths updated correctly.The character type imports have been successfully migrated to the new feature-scoped structure.
src/features/game/logic/typing/delete-word.ts (2)
1-1: LGTM! Import paths refactored correctly.The character type imports have been successfully updated to use the feature-scoped module path.
4-4: LGTM! Import path updated correctly.The snippet type import has been successfully migrated to the feature-scoped structure.
src/features/snippets/infrastructure/repositories/language.repository.server.ts (1)
1-2: Import paths updated correctly.The Prisma client and type imports have been migrated to the new module structure. The Prisma client path verification from the previous file review applies here as well.
src/features/game/logic/typing/add-character.ts (3)
3-4: LGTM! Import paths updated correctly.The type imports have been successfully migrated to the feature-scoped module structure.
5-5: Good addition! Immer improves immutability handling.Introducing Immer is a solid choice for managing immutable state updates. It provides cleaner syntax and better performance compared to manual deep cloning.
68-81: LGTM! Clean refactor using Immer'sproduce.The immutable update logic is correctly implemented using Immer's
produce. The draft mutation approach is more readable and less error-prone than manual cloning while maintaining the same behavior.src/features/snippets/infrastructure/repositories/snippet.repository.server.ts (1)
1-1: Import path verified and correctly configured.The new Prisma client path
@/server/prismaexists and exports theprismainstance with the requiredprismaRandom()extension applied. The export matches the import statement in the file, confirming the migration is valid.src/proxy.ts (1)
5-12: This review comment is based on incorrect assumptions about the code change.The premise that "previously, this check was skipped in development mode" has no factual basis. The code history shows:
- The internal API header check was introduced in April 2025 with commit
0c22d9900121333ed6a0e7e2e900a4e2cae7c689and applies uniformly across all environments—there is no development bypass logic present or recently removed.- The most recent change to
src/proxy.tswas a file structure refactor (not a logic change).- There are no environment-specific conditions (
process.env.NODE_ENV,isDev, etc.) in the proxy file.Development workflows are properly supported because the TRPC provider (configured in
src/components/providers/trpc-provider.tsxand used in the root layout) automatically sets the"x-internal-api": "true"header on all requests across all environments. This means the header requirement is transparent to developers and does not require special handling.Likely an incorrect or invalid review comment.
src/server/trpc/routers/snippet.ts (1)
8-10: Languages query looks goodThin wrapper over
getLanguages()is straightforward and keeps the router lean; types and async handling are correct.src/features/shared/types/snippet.ts (1)
1-9: Shared snippet types are clear and cohesiveThe split between
IServerSnippetand the client‑sideISnippet(withparsedSnippet) is well‑structured and will make game logic typing simpler.src/app/api/trpc/[trpc]/route.ts (1)
1-14: TRPC API handler wiring looks correct
fetchRequestHandleris configured properly withendpoint,appRouter, andcreateTRPCContext, and re‑exporting it as bothGETandPOSTaligns with tRPC’s expectations.src/server/trpc/trpc.ts (1)
1-8: Typed TRPC bootstrap is idiomaticUsing
initTRPC.context<TRPCContext>().create()and re‑exportingrouter,procedure, andmergeRouterskeeps router files clean and consistent.src/server/trpc/routers/index.ts (1)
1-8: Root appRouter composition is fineMounting
snippetRouterunderappRouter.snippetand exportingAppRouterfor client typing (createTRPCReact<AppRouter>()) is exactly what you need here.src/server/trpc/context.ts (1)
1-13: Context shape is minimal but correctReturning just
{ req }is fine for now and theTRPCContextalias viaAwaited<ReturnType<...>>will tolerate future additions (e.g. user/session) without ripple effects.src/app/game/page.tsx (2)
3-16: General refactor and stopwatch/snippet wiring look solid.Using
useGameSnippets,GameStatus, andbuildClientSnippethere simplifies the page nicely, and the renamedelapsedparameter inonTick/pushPositionSamplekeeps semantics clear while still reading the latest game-store state viauseGameStore.getState(). No issues spotted in these parts.Also applies to: 30-38, 40-42
112-145: Status/error gating keepsavailableLanguages!safe; UI branches are consistent.The
FINISHED,error, andLOADINGbranches ensure you don’t hit<GameView>untilstatus !== LOADINGand no error is present. Given that theGameStatus.LOADING→READYtransition only happens afterinitialize(which depends onavailableLanguages), the non-null assertionavailableLanguages!is effectively protected by this flow. This wiring looks coherent and doesn’t introduce obvious race conditions in the render path.
| const changeSnippet = async () => { | ||
| const snippetQueue = getSnippetQueue(); | ||
|
|
||
| const startTime = Date.now(); | ||
| setIsNextButtonLocked(true); | ||
|
|
||
| if (snippetQueue.length === 0) { | ||
| setStatus(GameStatus.LOADING); | ||
|
|
||
| if (backgroundFetchPromise.current) { | ||
| await backgroundFetchPromise.current; | ||
| backgroundFetchPromise.current = null; | ||
| } else if (language) { | ||
| await activateLanguage(language); | ||
| setIsNextButtonLocked(false); | ||
| return; | ||
| } | ||
| } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) { | ||
| backgroundFetchPromise.current = refillSnippetQueue(language!); | ||
| } | ||
|
|
||
| const elapsedTime = Date.now() - startTime; | ||
| const remainingTime = Math.max(REFRESH_BUTTON_MIN_DELAY - elapsedTime, 0); | ||
|
|
||
| goToNextSnippet(); | ||
|
|
||
| setTimeout(() => { | ||
| setIsNextButtonLocked(false); | ||
| }, remainingTime); | ||
| }; |
There was a problem hiding this comment.
Bug: queue exhaustion can throw when background refill has already completed
changeSnippet has a subtle queue/async bug:
- When the queue length drops to
<= 3, you start a background refill:} else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) { backgroundFetchPromise.current = refillSnippetQueue(language!); }
backgroundFetchPromise.currentis never reset tonullafter that promise resolves (unless you later hit thesnippetQueue.length === 0branch while it’s still truthy).- After consuming all snippets from both the initial batch and that refill,
getSnippetQueue()will eventually return an empty array, butbackgroundFetchPromise.currentwill still be a resolved promise. - On the next
changeSnippet()call with an empty queue:snippetQueue.length === 0is true.backgroundFetchPromise.currentis truthy, so youawaitit and then set it tonullwithout starting a new fetch.- The underlying store queue is still empty, and you then call
goToNextSnippet(), which will throw"Cannot go to next snippet when the queue is empty".
A targeted fix is to ensure the background promise clears itself when it finishes and to avoid treating an already‑resolved promise as “in‑flight”:
- if (snippetQueue.length === 0) {
- setStatus(GameStatus.LOADING);
-
- if (backgroundFetchPromise.current) {
- await backgroundFetchPromise.current;
- backgroundFetchPromise.current = null;
- } else if (language) {
- await activateLanguage(language);
- setIsNextButtonLocked(false);
- return;
- }
- } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current) {
- backgroundFetchPromise.current = refillSnippetQueue(language!);
- }
+ if (snippetQueue.length === 0) {
+ setStatus(GameStatus.LOADING);
+
+ if (backgroundFetchPromise.current) {
+ // Wait for any in‑flight refill to complete; the ref will be cleared in its .finally()
+ await backgroundFetchPromise.current;
+ } else if (language) {
+ await activateLanguage(language);
+ setIsNextButtonLocked(false);
+ return;
+ }
+ } else if (snippetQueue.length <= 3 && !backgroundFetchPromise.current && language) {
+ // Kick off a background refill and reset the ref when done
+ backgroundFetchPromise.current = refillSnippetQueue(language).finally(() => {
+ backgroundFetchPromise.current = null;
+ });
+ }This guarantees that once you’ve drained both the initial and refilled snippets, the next “empty queue” transition will either (a) wait for an actually in‑flight refill, or (b) re‑initialize via activateLanguage, instead of calling goToNextSnippet() on an empty queue.
🤖 Prompt for AI Agents
In src/features/game/hooks/useGameSnippets.ts around lines 48–77, the
changeSnippet flow treats a resolved backgroundFetchPromise as if it were still
"in-flight", causing an empty-queue call to goToNextSnippet and throw; fix by
ensuring backgroundFetchPromise.current is cleared when the refill promise
settles (assign the promise to backgroundFetchPromise.current but attach a
.finally handler that sets it to null) so awaiting it only represents an active
refill, and additionally after awaiting if the queue is still empty then trigger
the fallback path (e.g., call activateLanguage or set status/loading) instead of
immediately calling goToNextSnippet.
Summary by CodeRabbit
New Features
Architecture
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.