Conversation
… from index.tsx Split the monolithic index.tsx into focused modules to prepare for route-based loaders and file-per-route convention: - GlobalData: static accessor for window globals - routesBuilder: route path constants and type-safe URL builders - ErrorPage: react-router error boundary component - queryClient: shared module-level QueryClient instance - router.tsx: extracted createRouter() with full route tree - AppLayout.tsx: extracted layout component with provider nesting
📝 WalkthroughWalkthroughIntroduces a shared QueryClient and router factory (createRouter), extracts routing into route files and loaders, adds an AppLayout composing multiple providers (and an InstantiationUrlReset), centralises route patterns/URL builders, and adds route components/loaders that prefetch queries into the shared client. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RouterProvider
participant AppLayout
participant Loader
participant QueryClient
participant RouteComponent
Browser->>RouterProvider: navigate(path)
RouterProvider->>AppLayout: render matched route
AppLayout->>Loader: run route loader (if defined)
Loader->>QueryClient: prefetch queries (parties, instanceData, processState)
QueryClient-->>Loader: cached responses
Loader-->>AppLayout: return null
AppLayout->>RouteComponent: render via Outlet (with providers mounted)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
|
🔍 Lighthouse Report
Edited |
Create file-per-route structure under src/routes/ with dedicated route components and loaders that prefetch data via queryClient.ensureQueryData(). Wire loaders into router.tsx which now accepts a queryClient parameter. Lint fixes applied. Route folders: party-selection, instance-selection, instance, task, page, process-end, index — each with .route.tsx and .loader.ts files.
Route loaders handle all data prefetching (parties, instance, process) that AppPrefetcher previously did. AppQueriesProvider remains since 24 files still depend on useAppQueries/useAppMutations.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/App/frontend/src/routes/party-selection/party-selection.loader.ts (1)
7-10: Use the shared query key object instead of inline key definition.The inline
queryKey: ['parties', 'allowedToInstantiate']duplicates whatpartyQueryKeys.allowedToInstantiate()returns inPartiesProvider.tsx. Per coding guidelines, TanStack Query keys should be managed via shared objects to prevent drift.Additionally,
usePartiesQueryDefretrievesfetchPartiesAllowedToInstantiatefromuseAppQueries()context, but this loader imports it directly. If the context-provided function differs (e.g., different implementation or mocking), the prefetch may not match what the component queries.Consider exporting a
partyQueries.allowedToInstantiate(fetchFn)queryOptions object fromPartiesProvider.tsxand using it here, similar to howinstanceQueriesandprocessQueriesare used in other loaders.♻️ Suggested approach
import type { QueryClient } from '@tanstack/react-query'; -import { fetchPartiesAllowedToInstantiate } from 'src/queries/queries'; +import { partyQueries } from 'src/features/party/PartiesProvider'; export function partySelectionLoader(queryClient: QueryClient) { return function loader() { - queryClient.prefetchQuery({ - queryKey: ['parties', 'allowedToInstantiate'], - queryFn: fetchPartiesAllowedToInstantiate, - }); + queryClient.prefetchQuery(partyQueries.allowedToInstantiate()); return null; }; }This requires exporting a
partyQueriesobject withqueryOptionsfromPartiesProvider.tsx.As per coding guidelines: "Use objects for managing TanStack Query keys and functions, and use
queryOptionsfor sharing these across the system".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/frontend/src/routes/party-selection/party-selection.loader.ts` around lines 7 - 10, Replace the inline queryKey and direct import of fetchPartiesAllowedToInstantiate with the shared queryOptions from PartiesProvider: stop using ['parties','allowedToInstantiate'] and instead import partyQueries and call partyQueries.allowedToInstantiate(...) so the same key and options are used; obtain the fetch function from the app queries context (the same one used by usePartiesQueryDef) or pass it into the loader and supply it to partyQueries.allowedToInstantiate(fetchFn) so prefetchQuery uses the shared queryOptions (matching partyQueryKeys.allowedToInstantiate() and the context-provided fetchPartiesAllowedToInstantiate).src/App/frontend/src/routes/instance-selection/instance-selection.loader.ts (1)
5-12: Duplicate loader logic and inline query key.This loader is identical to
partySelectionLoaderinparty-selection.loader.ts— both prefetch the same['parties', 'allowedToInstantiate']query. Consider:
- Consolidate: If both routes need the same prefetch, extract a shared helper or reuse one loader.
- Use shared query objects: Same concern as
party-selection.loader.ts— the inline key should reference the sharedpartyQueryKeysobject to maintain consistency withusePartiesQueryDef.As per coding guidelines: "Use objects for managing TanStack Query keys and functions, and use
queryOptionsfor sharing these across the system".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/frontend/src/routes/instance-selection/instance-selection.loader.ts` around lines 5 - 12, The loader instanceSelectionLoader duplicates partySelectionLoader by inlining the same queryKey; refactor to reuse a shared query definition and key: replace the inline ['parties','allowedToInstantiate'] usage in instanceSelectionLoader with the shared partyQueryKeys (or the query object from usePartiesQueryDef) and/or export a common prefetch helper that calls queryClient.prefetchQuery with the shared queryOptions and fetchPartiesAllowedToInstantiate; update instanceSelectionLoader to call that shared helper or reuse partySelectionLoader so the TanStack Query key and options are managed from a single source of truth.src/App/frontend/src/router.tsx (1)
99-112: Avoid hard-coding the party-selection redirect target.This is the one redirect in the new router that bypasses the central route utilities. If that segment changes again, the legacy redirect can silently drift from the canonical path. Reuse the route builder/constant here instead of
'/party-selection'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/frontend/src/router.tsx` around lines 99 - 112, Replace the hard-coded '/party-selection' string used in the Navigate element for the fallback route (path '*') with the central route constant/builder from your route utilities (the same one used elsewhere in the app); update the Navigate to use that exported symbol (e.g., ROUTES.partySelection or buildRoute('partySelection')) so the fallback redirect (the Navigate element) always points at the canonical route instead of a literal string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App/frontend/AGENTS.md`:
- Around line 74-83: The verification checklist currently instructs running a
mutating command `yarn lint --fix`; change this to the non‑mutating `yarn lint`
in the AGENTS.md verification section so the step only verifies linting rather
than rewriting the worktree, and optionally mention `yarn lint --fix` as a
follow-up remediation step; update the text that references `yarn lint --fix` to
`yarn lint` and add a short note that `--fix` can be run separately if desired.
- Line 160: The AGENTS.md guidance block contains three typos; update the text
in the added paragraph (the sentence containing "con't") to "don't", and also
fix the later occurrences "delibaretely" to "deliberately" and "reoccuring" to
"recurring" so the guidance reads correctly; locate and edit the paragraph and
those words in AGENTS.md (search for the sentence about testing data processing
and the two later misspelled words) to apply these exact corrections.
In `@src/App/frontend/ROUTER_REFACTOR_PLAN.md`:
- Around line 65-86: The fenced code block in ROUTER_REFACTOR_PLAN.md is missing
a language identifier causing markdownlint to flag it; update the opening fence
for that block from ``` to ```text (or another appropriate language) so the
block starts with ```text and keep the closing fence as ``` to satisfy
markdownlint; locate the block containing the route tree (entries like
index.route.tsx, instance.route.tsx, task.route.tsx, etc.) and add the language
identifier there.
In `@src/App/frontend/src/AppLayout.tsx`:
- Around line 53-54: The current forEach uses an expression-bodied arrow that
returns the result of remove(...) causing the lint warning; change the callback
to a block-bodied arrow so it does not implicitly return a value — e.g., in the
code that calls queryClient.getMutationCache().findAll(...) and assigns
mutations, update the forEach callback for each mutation to use braces and a
statement body (e.g., mutations.forEach((mutation) => {
queryClient.getMutationCache().remove(mutation); });) so the intent is explicit
and the lint error is resolved while still calling remove on each mutation.
In `@src/App/frontend/src/routes/task/task.route.tsx`:
- Around line 10-15: The FixWrongReceiptType component calls
useCurrentDataModelDataElementId but FormProvider (which mounts
DataModelsProvider) is currently nested inside FixWrongReceiptType; move
FormProvider so it wraps FixWrongReceiptType (i.e., change the tree from
<FixWrongReceiptType><ProcessWrapper><FormProvider>... to
<FormProvider><FixWrongReceiptType><ProcessWrapper>...) ensuring
DataModelsProvider is mounted before FixWrongReceiptType runs.
---
Nitpick comments:
In `@src/App/frontend/src/router.tsx`:
- Around line 99-112: Replace the hard-coded '/party-selection' string used in
the Navigate element for the fallback route (path '*') with the central route
constant/builder from your route utilities (the same one used elsewhere in the
app); update the Navigate to use that exported symbol (e.g.,
ROUTES.partySelection or buildRoute('partySelection')) so the fallback redirect
(the Navigate element) always points at the canonical route instead of a literal
string.
In `@src/App/frontend/src/routes/instance-selection/instance-selection.loader.ts`:
- Around line 5-12: The loader instanceSelectionLoader duplicates
partySelectionLoader by inlining the same queryKey; refactor to reuse a shared
query definition and key: replace the inline ['parties','allowedToInstantiate']
usage in instanceSelectionLoader with the shared partyQueryKeys (or the query
object from usePartiesQueryDef) and/or export a common prefetch helper that
calls queryClient.prefetchQuery with the shared queryOptions and
fetchPartiesAllowedToInstantiate; update instanceSelectionLoader to call that
shared helper or reuse partySelectionLoader so the TanStack Query key and
options are managed from a single source of truth.
In `@src/App/frontend/src/routes/party-selection/party-selection.loader.ts`:
- Around line 7-10: Replace the inline queryKey and direct import of
fetchPartiesAllowedToInstantiate with the shared queryOptions from
PartiesProvider: stop using ['parties','allowedToInstantiate'] and instead
import partyQueries and call partyQueries.allowedToInstantiate(...) so the same
key and options are used; obtain the fetch function from the app queries context
(the same one used by usePartiesQueryDef) or pass it into the loader and supply
it to partyQueries.allowedToInstantiate(fetchFn) so prefetchQuery uses the
shared queryOptions (matching partyQueryKeys.allowedToInstantiate() and the
context-provided fetchPartiesAllowedToInstantiate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bf02f80-ad68-407d-95a1-da697b1434f2
📒 Files selected for processing (22)
src/App/frontend/AGENTS.mdsrc/App/frontend/CLAUDE.mdsrc/App/frontend/ROUTER_REFACTOR_PLAN.mdsrc/App/frontend/src/AppLayout.tsxsrc/App/frontend/src/GlobalData.tssrc/App/frontend/src/core/contexts/AppQueriesProvider.tsxsrc/App/frontend/src/index.tsxsrc/App/frontend/src/queryClient.tssrc/App/frontend/src/router.tsxsrc/App/frontend/src/routes/index/index.loader.tssrc/App/frontend/src/routes/index/index.route.tsxsrc/App/frontend/src/routes/instance-selection/instance-selection.loader.tssrc/App/frontend/src/routes/instance-selection/instance-selection.route.tsxsrc/App/frontend/src/routes/instance/instance.loader.tssrc/App/frontend/src/routes/instance/instance.route.tsxsrc/App/frontend/src/routes/page/page.route.tsxsrc/App/frontend/src/routes/party-selection/party-selection.loader.tssrc/App/frontend/src/routes/party-selection/party-selection.route.tsxsrc/App/frontend/src/routes/process-end/process-end.route.tsxsrc/App/frontend/src/routes/task/task.loader.tssrc/App/frontend/src/routes/task/task.route.tsxsrc/App/frontend/src/routesBuilder.ts
olemartinorg
left a comment
There was a problem hiding this comment.
Some tests failing around party selection, but I assume you'll fix those before merging. 🙌
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/App/frontend/src/features/instantiate/containers/PartySelection.tsx (1)
121-129: Use a feature-specific test id here.
presentationis too generic for a stable selector and is likely to collide with other layout wrappers over time. If this is meant for tests, prefer something scoped to this screen, e.g.party-selectionorparty-selection-header.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App/frontend/src/features/instantiate/containers/PartySelection.tsx` around lines 121 - 129, Replace the generic data-testid on the Flex wrapper in the PartySelection component: change the attribute data-testid='presentation' (inside the Flex in PartySelection.tsx) to a feature-scoped id such as data-testid='party-selection' or data-testid='party-selection-header' so tests use a stable, screen-specific selector; update any tests that rely on the old 'presentation' id to the new id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App/frontend/test/e2e/integration/stateless-app/party-selection.ts`:
- Line 10: The test uses an undefined test wrapper named "itq" which will throw
a runtime error; replace the "itq" call with the standard Cypress test function
"it" in the test definition (or, if "itq" was intentionally a custom wrapper,
add a proper definition/import for "itq" and ensure it mirrors the expected
behavior), updating the invocation in the party-selection test so it uses a
valid test function.
---
Nitpick comments:
In `@src/App/frontend/src/features/instantiate/containers/PartySelection.tsx`:
- Around line 121-129: Replace the generic data-testid on the Flex wrapper in
the PartySelection component: change the attribute data-testid='presentation'
(inside the Flex in PartySelection.tsx) to a feature-scoped id such as
data-testid='party-selection' or data-testid='party-selection-header' so tests
use a stable, screen-specific selector; update any tests that rely on the old
'presentation' id to the new id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b01ea893-574f-4e86-b403-b757ddb4f16d
📒 Files selected for processing (2)
src/App/frontend/src/features/instantiate/containers/PartySelection.tsxsrc/App/frontend/test/e2e/integration/stateless-app/party-selection.ts
extract router, AppLayout, queryClient, and route utilities from index.tsx
Split the monolithic index.tsx into focused modules to prepare for route-based loaders and file-per-route convention:
Description
Verification
Summary by CodeRabbit
New Features
Refactor
Tests