fix(react): isolate loader data from activity context - #736
fix(react): isolate loader data from activity context#736ENvironmentSet wants to merge 10 commits into
Conversation
🦋 Changeset detectedLatest commit: 505c264 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughLoader results are stored in loader-plugin-owned runtime state and referenced by serializable IDs in activity context. A React context supplies the associated promise to activities, while initialization, restoration, and route preloading stage and resolve results through the shared map. ChangesLoader data isolation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RouteNavigation
participant loaderPlugin
participant loaderResults
participant LoaderDataProvider
participant Activity
RouteNavigation->>loaderPlugin: Start before-route loading
loaderPlugin->>loaderPlugin: actions.pause()
loaderPlugin->>loaderResults: Store loader promise by loaderResultId
loaderPlugin->>loaderPlugin: actions.overrideActionParams()
loaderPlugin->>loaderPlugin: actions.resume()
loaderPlugin->>LoaderDataProvider: Provide loader promise
LoaderDataProvider->>Activity: Render activity with loader context
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
@stackflow/link
@stackflow/plugin-basic-ui
@stackflow/plugin-blocker
@stackflow/plugin-google-analytics-4
@stackflow/plugin-history-sync
@stackflow/plugin-lifecycle
@stackflow/plugin-renderer-basic
@stackflow/plugin-renderer-web
@stackflow/react-ui-core
@stackflow/react
commit: |
Deploying stackflow-demo with
|
| Latest commit: |
505c264
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0a1f69c3.stackflow-demo.pages.dev |
| Branch Preview URL: | https://feature-fep-2613.stackflow-demo.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 505c264 | Commit Preview URL | Jul 30 2026, 06:52 AM |
3058ce7 to
87286ba
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
integrations/react/src/loader/loaderPlugin.tsx (1)
328-339: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRetain staged loader data until a paused action is promoted or canceled.
When the loader or lazy component is pending,
actions.pause()defers the route, but this unconditional next-microtask cleanup can delete the only runtime loader entry beforeactions.resume()triggersonChangedpromotion. The eventual activity then receives no loader promise throughLoaderDataProvider, breakinguseLoaderData()for deferred navigations. Clean this entry only after confirmed cancellation/unreached dispatch, or after promotion on resume.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/react/src/loader/loaderPlugin.tsx` around lines 328 - 339, Update the staged loader-data cleanup around runtimeLoaderDataByActivityId so pending actions paused by actions.pause() retain their entry until actions.resume() promotes it or the action is confirmed canceled/unreached. Remove the unconditional next-microtask deletion, and clean entries after cancellation or successful onChanged promotion while preserving cleanup for dispatches that never reach the stack.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@integrations/react/src/loader/loaderPlugin.tsx`:
- Around line 328-339: Update the staged loader-data cleanup around
runtimeLoaderDataByActivityId so pending actions paused by actions.pause()
retain their entry until actions.resume() promotes it or the action is confirmed
canceled/unreached. Remove the unconditional next-microtask deletion, and clean
entries after cancellation or successful onChanged promotion while preserving
cleanup for dispatches that never reach the stack.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f4829b6-0c85-48e8-872a-3dce68d5f184
📒 Files selected for processing (1)
integrations/react/src/loader/loaderPlugin.tsx
1 participant
Problem
The built-in
loaderPluginin@stackflow/reactstores loader results (as Promises) in the sharedactivityContext.loaderData. SinceactivityContextis shared across all plugins and is included verbatim inStackSnapshotevents:loaderDatakey can collide with other plugins' or users' data and be silently overwritten depending on execution order.StackSnapshot.Solution
Loader runtime state now lives in plugin-owned storage, fully outside
activityContext:loaderPlugininstance owns private Maps keyed by the entering domain-event ID, isolating multiple Stackflow/SSR store instances and keeping separate loader generations whenreplace()reuses an activity ID (including a rendered generation plus a queued paused replacement).wrapActivityprovides the promise foractivity.enteredBy.idtouseLoaderDatathrough an internal React Context — no render-phase mutation, no effects.exit-donegenerations, retains queued paused generations, and removes staged data for prevented actions.loaderPluginno longer reads or writesactivityContext.loaderData; values placed there by users or other plugins pass through unchanged (and are no longer consumed byuseLoaderData).No
@stackflow/corepublic contract changed. Existing public surfaces are preserved:useLoaderData, SSRinitialLoaderData, core v3load(restore) path with loader re-execution and deferred resolution,pausedEventshandling, pause/resume loading behavior, and the lazyshouldRenderImmediatelypath.Verification
yarn typecheck(all workspaces),@stackflow/react/@stackflow/plugin-stack-persistence/ demo builds — PASS@stackflow/core(14 suites / 84 tests),@stackflow/plugin-history-sync(7 suites / 52 tests) — PASSuseLoaderDatarender, foreign-key collision isolation, Snapshot JSON safety, core v3 restore round-trip, SSRinitialLoaderData, plugin-stack-persistence save/load round-trip, paused same-ID replacement generations, pending loader + lazy pause/resume, independent stores reusing identical IDs, exit-done / prevented-action cleanup — all PASS. The harness rejects any Promise found in captured Snapshot records and confirms deliberately-planted foreignactivityContext.loaderDatavalues are preserved for core/persistence whileuseLoaderDatareturns only the plugin-owned value.Closes FEP-2613
🤖 Generated with Claude Code
https://claude.ai/code/session_01EYjxc3pkrpHin2PhoVQABq