feat(react): add loader data invalidation support#681
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…port - Add ActivityLoaderConfig union type (function | object with fn + shouldInvalidate) - Add getLoaderFn and getShouldInvalidate utility functions - Update ActivityDefinition to use ActivityLoaderConfig Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create ActivityLoaderContext for per-activity loader data management - Create ActivityLoaderProvider with Core store subscription - Implement useLoader hook with runtime loader validation - Add wrapActivity to loaderPlugin for provider wrapping - Update useLoaderData to prefer context (backward compatible) - Export useLoader from future API Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ties Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughIntroduces a loader data invalidation system across Stackflow's configuration and React integration. Adds ActivityLoaderConfig union type supporting both function and object-based loader specifications with optional shouldInvalidate callbacks. Implements React context, hooks, and provider components for per-activity loader data management with pluggable invalidation policies. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Component
participant Hook as useLoader Hook
participant Context as ActivityLoaderContext
participant Provider as ActivityLoaderProvider
participant Core as Core Store
participant Loader as Loader Function
User->>Hook: useLoader({ loaderFn })
Hook->>Context: useContext(ActivityLoaderContext)
Hook->>Hook: Validate loaderFn against config
Note over Provider: On Activity Change
Core->>Provider: Subscribe to stack changes
Core-->>Provider: Activity updated
Provider->>Provider: Call shouldInvalidate?
alt shouldInvalidate returns true
Provider->>Loader: Reload data
Loader-->>Provider: Return new data
Provider->>Context: Update loaderData
end
Hook-->>User: { data, invalidate }
User->>Hook: Call invalidate()
Hook->>Provider: invalidate()
Provider->>Loader: Reload data
Loader-->>Provider: Return new data
Provider->>Context: Update loaderData
Context-->>User: Component re-renders with new data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
commit: |
Deploying stackflow-demo with
|
| Latest commit: |
b7658c4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6859a728.stackflow-demo.pages.dev |
| Branch Preview URL: | https://vk-6a7c-loader-data-inva.stackflow-demo.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | b7658c4 | Commit Preview URL | Feb 06 2026, 09:55 AM |
- Add LoaderOptions interface with shouldInvalidate - Add function overloads for loader() - Return ActivityLoaderConfigObject when options provided - Add tests for loader function Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/plans/2026-02-06-loader-invalidation-impl.md`:
- Around line 15-118: Missing test step: add a unit test file for the new
utilities; create config/src/ActivityLoader.spec.ts that tests getLoaderFn and
getShouldInvalidate (covering undefined, function config, and object config
cases), run tests (cd config && yarn test) to ensure they pass, and include the
spec file in the git add/commit alongside ActivityLoader.ts,
ActivityDefinition.ts, and index.ts so the plan and commit are consistent.
- Around line 733-739: The Article component uses React's top-level use hook
(import { use } from "react") to unwrap the loader promise (const resolved =
use(data)), which requires React 19 but the demo declares React 18.3.1; fix by
either bumping the demo's React dependency to 19+ in package.json or remove the
use hook and switch Article to a React 18-compatible pattern (e.g., keep
useLoader({ loaderFn: articleLoader }) and render using Suspense + an async
resource handling or await the loader result in an effect), updating references
to use, resolved, and useLoader accordingly so the component no longer calls use
from React 19 when running under 18.x.
In `@integrations/react/src/future/loader/ActivityLoaderProvider.tsx`:
- Around line 36-65: The issue is prevActivityRef can be stale after the effect
re-subscribes; before calling subscribe in the useEffect, initialize
prevActivityRef.current to the current activity from actions.getStack() (e.g.,
find by activity.id) or null so the first subscription callback uses an
up-to-date prevActivity; keep the rest of the subscribe callback logic
(shouldInvalidate, loadData, setLoaderData) unchanged and ensure you still
update prevActivityRef.current inside the callback after handling invalidation.
🧹 Nitpick comments (10)
integrations/react/src/future/loader/ActivityLoaderProvider.tsx (2)
67-71: Memoize the context value to avoid unnecessary consumer re-renders.The
{ loaderData, invalidate }object literal creates a new reference on every render, causing all context consumers to re-render even when neither field has changed.♻️ Proposed fix
-import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react";+ const contextValue = useMemo( + () => ({ loaderData, invalidate }), + [loaderData, invalidate], + ); + return ( - <ActivityLoaderContext.Provider value={{ loaderData, invalidate }}> + <ActivityLoaderContext.Provider value={contextValue}> {children} </ActivityLoaderContext.Provider> );
11-11:activityParamstyped as{}is overly permissive.The empty object type
{}in TypeScript matches any non-nullish value, which disables meaningful type checking. Consider usingRecord<string, string>or aligning with the params type used elsewhere in the codebase.docs/plans/2026-02-04-loader-invalidation-design.md (1)
29-42: Add a language identifier to the fenced code block.The ASCII diagram block is missing a language specifier, which triggers a markdownlint warning (MD040). Use
textsince it's not actual code.Proposed fix
-``` +```text ┌─────────────────────────────────────────────────────┐integrations/react/src/future/loader/useLoaderData.ts (1)
14-14: Nit: Consider translating the Korean comment to English.This improves readability for all contributors. Something like:
// Prefer loader data from ActivityLoaderProvider context; fall back to activity.context.integrations/react/src/future/loader/useLoader.ts (1)
25-35: Runtime validation is sound but consider the developer experience for the error path.The
findon line 25 silently returnsundefinedwhen the activity isn't in the config, which then causesconfigLoaderFnto beundefined, and the mismatch error on line 30 fires with a somewhat misleading message — it says "loader mismatch" when the real problem may be that no loader was configured at all for the activity (or the activity name wasn't found in config).Consider distinguishing these cases for better DX:
Proposed improvement
const activityConfig = config.activities.find( (a) => a.name === activity.name, ); const configLoaderFn = getLoaderFn(activityConfig?.loader); + if (!configLoaderFn) { + throw new Error( + `No loader is configured for "${activity.name}" activity. ` + + `Ensure a loader is registered in the config before using useLoader().`, + ); + } + if (options.loaderFn !== configLoaderFn) { throw new Error( `Loader mismatch: the provided loader does not match the loader ` + `registered for "${activity.name}" activity in the config.`, ); }integrations/react/src/future/loader/loaderPlugin.tsx (1)
81-85:loadervariable is unused beyond the guard check — consider renaming for clarity.
loader(the resolved function) is obtained on line 81 but only used as a boolean guard on line 83. It's never invoked —loadDatais called instead on line 87. This is functionally correct (you're checking if a loader is configured), but naming itloadersuggests it will be used. A small rename would clarify intent.Optional rename
- const loader = getLoaderFn(matchActivity?.loader); + const hasLoader = !!getLoaderFn(matchActivity?.loader); - if (!loader || !matchActivity) { + if (!hasLoader || !matchActivity) {docs/plans/2026-02-06-loader-invalidation-impl.md (4)
181-184: Consider stability ofshouldInvalidateprop.The
shouldInvalidatecallback is included in the effect's dependency array (line 233), but if this prop is not stable between renders, it will cause the subscription to be recreated unnecessarily. Consider documenting thatshouldInvalidateshould be extracted as a stable reference from the config, or wrap it withuseCallbackif it's computed.💡 Alternative: use a ref for shouldInvalidate
export function ActivityLoaderProvider({ activity, initialLoaderData, loadData, shouldInvalidate, children, }: ActivityLoaderProviderProps) { const [loaderData, setLoaderData] = useState(initialLoaderData); const actions = useCoreActions(); const prevActivityRef = useRef<Activity>(activity); + const shouldInvalidateRef = useRef(shouldInvalidate); + + useEffect(() => { + shouldInvalidateRef.current = shouldInvalidate; + }); // ... rest of code useEffect(() => { - if (!shouldInvalidate) { + if (!shouldInvalidateRef.current) { return; } const unsubscribe = actions.subscribe(() => { // ... - if (shouldInvalidate({ prevActivity, currentActivity })) { + if (shouldInvalidateRef.current?.({ prevActivity, currentActivity })) { // ... } }); return unsubscribe; - }, [actions, activity.id, loadData, shouldInvalidate]); + }, [actions, activity.id, loadData]);This prevents unnecessary re-subscriptions when shouldInvalidate reference changes.
300-311: Reference equality check for loader validation may be fragile.The runtime validation uses strict equality (
options.loaderFn !== configLoaderFn) to compare loader functions. This will fail if the loader function reference changes due to wrapping, HOCs, or recreation. While this should work for the typical case where loaders are defined as module-level constants, consider documenting this limitation or using a more robust validation approach.Potential alternatives:
- Document that loaders must be stable references (simplest)
- Compare by
function.nameproperty (less reliable but more flexible)- Add a symbol or ID to loader functions for identification
For most use cases, the current approach with proper documentation should suffice.
384-386: Unnecessary fragment wrapper.The fragment
<>...</>wrapper is unnecessary when returningactivity.render()directly. While harmless, it adds an extra node to the tree.♻️ Simplify return statement
if (!matchActivity?.loader) { - return <>{activity.render()}</>; + return activity.render(); }
580-596: Reorder mock definitions for clarity.The
useConfigmock (lines 586-594) referencesmockLoaderbefore it's defined (line 596). While JavaScript hoisting will handle this, it's clearer to definemockLoaderfirst.♻️ Reorder for better readability
+const mockLoader = vi.fn(() => Promise.resolve({ title: "Test" })); + // Mock dependencies vi.mock("../../stable", () => ({ useActivity: vi.fn(), })); vi.mock("../config/useConfig", () => ({ useConfig: vi.fn(() => ({ activities: [ { name: "TestActivity", loader: mockLoader, }, ], })), })); -const mockLoader = vi.fn(() => Promise.resolve({ title: "Test" }));
| ## Task 1: ActivityLoaderConfig 타입 정의 | ||
|
|
||
| **Files:** | ||
| - Modify: `config/src/ActivityLoader.ts` | ||
| - Modify: `config/src/ActivityDefinition.ts` | ||
| - Modify: `config/src/index.ts` | ||
|
|
||
| **Step 1: ActivityLoaderConfig 타입 추가** | ||
|
|
||
| `config/src/ActivityLoader.ts` 파일 끝에 추가: | ||
|
|
||
| ```typescript | ||
| import type { Activity } from "@stackflow/core"; | ||
|
|
||
| export interface ActivityLoaderConfigObject< | ||
| ActivityName extends RegisteredActivityName, | ||
| > { | ||
| fn: ActivityLoader<ActivityName>; | ||
| shouldInvalidate?: (args: { | ||
| prevActivity: Activity; | ||
| currentActivity: Activity; | ||
| }) => boolean; | ||
| } | ||
|
|
||
| export type ActivityLoaderConfig<ActivityName extends RegisteredActivityName> = | ||
| | ActivityLoader<ActivityName> | ||
| | ActivityLoaderConfigObject<ActivityName>; | ||
| ``` | ||
|
|
||
| **Step 2: 유틸리티 함수 추가** | ||
|
|
||
| `config/src/ActivityLoader.ts` 파일에 추가: | ||
|
|
||
| ```typescript | ||
| export function getLoaderFn<ActivityName extends RegisteredActivityName>( | ||
| loaderConfig: ActivityLoaderConfig<ActivityName> | undefined, | ||
| ): ActivityLoader<ActivityName> | undefined { | ||
| if (!loaderConfig) { | ||
| return undefined; | ||
| } | ||
| if (typeof loaderConfig === "function") { | ||
| return loaderConfig; | ||
| } | ||
| return loaderConfig.fn; | ||
| } | ||
|
|
||
| export function getShouldInvalidate<ActivityName extends RegisteredActivityName>( | ||
| loaderConfig: ActivityLoaderConfig<ActivityName> | undefined, | ||
| ): ActivityLoaderConfigObject<ActivityName>["shouldInvalidate"] | undefined { | ||
| if (!loaderConfig || typeof loaderConfig === "function") { | ||
| return undefined; | ||
| } | ||
| return loaderConfig.shouldInvalidate; | ||
| } | ||
| ``` | ||
|
|
||
| **Step 3: ActivityDefinition 타입 수정** | ||
|
|
||
| `config/src/ActivityDefinition.ts` 파일의 `loader` 프로퍼티 타입 변경: | ||
|
|
||
| ```typescript | ||
| import type { ActivityLoaderConfig } from "./ActivityLoader"; | ||
|
|
||
| export interface ActivityDefinition< | ||
| ActivityName extends RegisteredActivityName, | ||
| > { | ||
| name: ActivityName; | ||
| loader?: ActivityLoaderConfig<any>; // ActivityLoader에서 ActivityLoaderConfig로 변경 | ||
| } | ||
| ``` | ||
|
|
||
| **Step 4: index.ts에서 export 추가** | ||
|
|
||
| `config/src/index.ts`에 추가: | ||
|
|
||
| ```typescript | ||
| export type { | ||
| ActivityLoaderConfig, | ||
| ActivityLoaderConfigObject, | ||
| } from "./ActivityLoader"; | ||
| export { getLoaderFn, getShouldInvalidate } from "./ActivityLoader"; | ||
| ``` | ||
|
|
||
| **Step 5: 빌드 확인** | ||
|
|
||
| Run: `cd config && yarn build` | ||
| Expected: 빌드 성공 | ||
|
|
||
| **Step 6: Commit** | ||
|
|
||
| ```bash | ||
| git add config/src/ActivityLoader.ts config/src/ActivityDefinition.ts config/src/index.ts | ||
| git commit -m "$(cat <<'EOF' | ||
| feat(config): add ActivityLoaderConfig type with shouldInvalidate support | ||
|
|
||
| - Add ActivityLoaderConfig union type (function | object with fn + shouldInvalidate) | ||
| - Add getLoaderFn and getShouldInvalidate utility functions | ||
| - Update ActivityDefinition to use ActivityLoaderConfig | ||
|
|
||
| Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> | ||
| EOF | ||
| )" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Missing test file creation step for config utilities.
The AI summary indicates that config/src/ActivityLoader.spec.ts was created with tests for getLoaderFn and getShouldInvalidate, but Task 1 doesn't include a step to create this test file. This creates an inconsistency in the implementation plan.
📝 Suggested addition after Step 5
Add a new step before the final commit:
**Step 5.5: Add tests for utility functions**
Create `config/src/ActivityLoader.spec.ts`:
```typescript
import { describe, expect, it } from "vitest";
import { getLoaderFn, getShouldInvalidate } from "./ActivityLoader";
describe("getLoaderFn", () => {
it("should return undefined for undefined config", () => {
expect(getLoaderFn(undefined)).toBeUndefined();
});
it("should return the function when config is a function", () => {
const fn = () => {};
expect(getLoaderFn(fn)).toBe(fn);
});
it("should return fn property when config is an object", () => {
const fn = () => {};
expect(getLoaderFn({ fn })).toBe(fn);
});
});
describe("getShouldInvalidate", () => {
it("should return undefined for undefined config", () => {
expect(getShouldInvalidate(undefined)).toBeUndefined();
});
it("should return undefined when config is a function", () => {
expect(getShouldInvalidate(() => {})).toBeUndefined();
});
it("should return shouldInvalidate when config is an object", () => {
const shouldInvalidate = () => true;
expect(getShouldInvalidate({ fn: () => {}, shouldInvalidate })).toBe(shouldInvalidate);
});
});
Run: `cd config && yarn test`
Expected: Tests pass
🤖 Prompt for AI Agents
In `@docs/plans/2026-02-06-loader-invalidation-impl.md` around lines 15 - 118,
Missing test step: add a unit test file for the new utilities; create
config/src/ActivityLoader.spec.ts that tests getLoaderFn and getShouldInvalidate
(covering undefined, function config, and object config cases), run tests (cd
config && yarn test) to ensure they pass, and include the spec file in the git
add/commit alongside ActivityLoader.ts, ActivityDefinition.ts, and index.ts so
the plan and commit are consistent.
| import { use } from "react"; | ||
| import { useLoader } from "@stackflow/react/future"; | ||
| import { articleLoader } from "./Article.loader"; | ||
|
|
||
| export const Article = () => { | ||
| const { data, invalidate } = useLoader({ loaderFn: articleLoader }); | ||
| const resolved = use(data); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find demo app package.json files
find . -name "package.json" -path "*/demo*" -type f | head -20Repository: daangn/stackflow
Length of output: 79
🏁 Script executed:
#!/bin/bash
# Read the React version from demo/package.json
cat ./demo/package.json | jq '.dependencies.react, .devDependencies.react'Repository: daangn/stackflow
Length of output: 74
React version incompatibility: use hook requires React 19+, but demo app specifies React 18.3.1.
The demo code uses React's use hook to unwrap promises (lines 733, 739), which was introduced in React 19. The demo app's package.json specifies "^18.3.1", which does not support this hook. Either upgrade the React dependency to 19 or use an alternative pattern such as Suspense with async/await.
🤖 Prompt for AI Agents
In `@docs/plans/2026-02-06-loader-invalidation-impl.md` around lines 733 - 739,
The Article component uses React's top-level use hook (import { use } from
"react") to unwrap the loader promise (const resolved = use(data)), which
requires React 19 but the demo declares React 18.3.1; fix by either bumping the
demo's React dependency to 19+ in package.json or remove the use hook and switch
Article to a React 18-compatible pattern (e.g., keep useLoader({ loaderFn:
articleLoader }) and render using Suspense + an async resource handling or await
the loader result in an effect), updating references to use, resolved, and
useLoader accordingly so the component no longer calls use from React 19 when
running under 18.x.
| useEffect(() => { | ||
| if (!shouldInvalidate) { | ||
| return; | ||
| } | ||
|
|
||
| const unsubscribe = subscribe(() => { | ||
| const stack = actions.getStack(); | ||
| const currentActivity = stack.activities.find( | ||
| (a) => a.id === activity.id, | ||
| ); | ||
|
|
||
| if (!currentActivity) { | ||
| return; | ||
| } | ||
|
|
||
| const prevActivity = prevActivityRef.current; | ||
|
|
||
| if (shouldInvalidate({ prevActivity, currentActivity })) { | ||
| const newLoaderData = loadData( | ||
| currentActivity.name, | ||
| currentActivity.params, | ||
| ); | ||
| setLoaderData(newLoaderData); | ||
| } | ||
|
|
||
| prevActivityRef.current = currentActivity; | ||
| }); | ||
|
|
||
| return unsubscribe; | ||
| }, [actions, subscribe, activity.id, loadData, shouldInvalidate]); |
There was a problem hiding this comment.
prevActivityRef may become stale after effect re-subscription.
When the useEffect deps change (e.g., shouldInvalidate or loadData reference changes), the subscription tears down and re-creates. However, prevActivityRef is not reset — it retains whatever value was last written by the previous subscription. On the first callback of the new subscription, shouldInvalidate may compare against a stale prevActivity from before the teardown, potentially producing an incorrect invalidation decision.
Consider resetting the ref when the effect re-runs:
Proposed fix
useEffect(() => {
if (!shouldInvalidate) {
return;
}
+ prevActivityRef.current = activity;
+
const unsubscribe = subscribe(() => {📝 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.
| useEffect(() => { | |
| if (!shouldInvalidate) { | |
| return; | |
| } | |
| const unsubscribe = subscribe(() => { | |
| const stack = actions.getStack(); | |
| const currentActivity = stack.activities.find( | |
| (a) => a.id === activity.id, | |
| ); | |
| if (!currentActivity) { | |
| return; | |
| } | |
| const prevActivity = prevActivityRef.current; | |
| if (shouldInvalidate({ prevActivity, currentActivity })) { | |
| const newLoaderData = loadData( | |
| currentActivity.name, | |
| currentActivity.params, | |
| ); | |
| setLoaderData(newLoaderData); | |
| } | |
| prevActivityRef.current = currentActivity; | |
| }); | |
| return unsubscribe; | |
| }, [actions, subscribe, activity.id, loadData, shouldInvalidate]); | |
| useEffect(() => { | |
| if (!shouldInvalidate) { | |
| return; | |
| } | |
| prevActivityRef.current = activity; | |
| const unsubscribe = subscribe(() => { | |
| const stack = actions.getStack(); | |
| const currentActivity = stack.activities.find( | |
| (a) => a.id === activity.id, | |
| ); | |
| if (!currentActivity) { | |
| return; | |
| } | |
| const prevActivity = prevActivityRef.current; | |
| if (shouldInvalidate({ prevActivity, currentActivity })) { | |
| const newLoaderData = loadData( | |
| currentActivity.name, | |
| currentActivity.params, | |
| ); | |
| setLoaderData(newLoaderData); | |
| } | |
| prevActivityRef.current = currentActivity; | |
| }); | |
| return unsubscribe; | |
| }, [actions, subscribe, activity.id, loadData, shouldInvalidate]); |
🤖 Prompt for AI Agents
In `@integrations/react/src/future/loader/ActivityLoaderProvider.tsx` around lines
36 - 65, The issue is prevActivityRef can be stale after the effect
re-subscribes; before calling subscribe in the useEffect, initialize
prevActivityRef.current to the current activity from actions.getStack() (e.g.,
find by activity.id) or null so the first subscription callback uses an
up-to-date prevActivity; keep the rest of the subscribe callback logic
(shouldInvalidate, loadData, setLoaderData) unchanged and ensure you still
update prevActivityRef.current inside the callback after handling invalidation.
1 participant
Summary
useLoader()hook that returns{ data, invalidate }for explicit loader invalidation controlshouldInvalidatecallback option for activity state-based automatic invalidationuseLoaderData()APIChanges
@stackflow/config
ActivityLoaderConfigunion type (Function | { fn, shouldInvalidate })getLoaderFn()andgetShouldInvalidate()utility functions@stackflow/react
ActivityLoaderContextfor per-activity loader data managementActivityLoaderProviderwith Core store subscription for state change detectionuseLoader()hook with runtime loader validationloaderPluginwithwrapActivityhook to wrap activities with provideruseLoaderData()to prefer context when available (backward compatible)Usage