docs(journeys): document multi-journey persistence patterns and the shared-singleton trap#42
docs(journeys): document multi-journey persistence patterns and the shared-singleton trap#42kibertoad wants to merge 3 commits into
Conversation
Shells registering more than one journey often share a single persistence setup expression across every `registerJourney` call. The natural first attempt — a single `<unknown, TInput>`-typed adapter object reused everywhere — fails the variance check on `JourneyDefinition.start(state: TState, ...)` with `Type 'unknown' is not assignable to type 'YourJourneyState'`, and the symptom points at the `persistence` arg rather than the underlying widening. Add a "Sharing a persistence config across multiple journeys" subsection under Persistence that names the anti-pattern, explains the variance failure, and shows the generic-factory recipe (per-journey `makeJourneyPersistence<TInput, TState>()` invocations) used by adopters in the wild. Composes with the existing `defineJourneyPersistence` and `createWebStoragePersistence` paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Audit of the existing examples showed the journey-invoke samples already document a second valid pattern — N distinct typed adapters, one per journey — which is arguably the simpler answer when the setup expression is short. The first draft only taught the generic-factory shape and risked implying it was the only fix. Restructure the section into three explicit headings: "Safe — one typed adapter per journey" (with a pointer at the journey-invoke examples), "Safe — a generic factory" (the previous content), and "Trap — one shared adapter typed `<unknown, TInput>`" (the variance-failure case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded a new "Registering multiple journeys" persistence subsection to the README explaining safe patterns for wiring multiple journey adapters, including a typed-per-journey approach and a generic factory pattern, with example code and documentation of a common TypeScript typing trap. ChangesMultiple Journey Persistence Guidance
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/journeys/README.md`:
- Around line 1664-1700: Remove the duplicated safe-factory block (the repeated
code that defines makeJourneyPersistence and the two registry.registerJourney
calls) and replace it with a short anti-pattern example that demonstrates the
variance failure; show a failing example using createWebStoragePersistence and
SyncJourneyPersistence with a contravariant/wrong TState (e.g. const
sharedPersistence: SyncJourneyPersistence<unknown, AppJourneyKey> =
createWebStoragePersistence<AppJourneyKey, unknown>({...})) and then show
registry.registerJourney(onboardingJourney, { persistence: sharedPersistence })
producing the type error, while keeping the original safe patterns above
(makeJourneyPersistence, createWebStoragePersistence, registry.registerJourney)
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46d32326-ad23-4ac3-a7af-0986436483ec
📒 Files selected for processing (1)
packages/journeys/README.md
CodeRabbit flagged that the "Trap" subsection's code block was a copy of the safe-factory example above it instead of the variance failure it's describing. Replace it with the actual shared `SyncJourneyPersistence<unknown, AppJourneyKey>` shape and the two `registerJourney` calls that error on the `persistence` arg. Also drop the trailing factory-pattern paragraph that duplicated the one above the trap.
1 participant
Summary
Add a "Registering multiple journeys" subsection under Persistence in
packages/journeys/README.md. It names the variance trap that shows up when a shell tries to DRY its persistence setup across multiple journeys by typing one shared adapter<unknown, TInput>, and walks through the two safe shapes already used in adopters and in this repo's own examples.Why
The README currently teaches persistence in the context of a single journey, and the existing examples either persist one journey (
customer-onboarding) or declare distinct typed adapters per journey (journey-invoke). Both shapes are safe, but neither warns about what happens when a multi-journey shell author writes the natural-looking shared singleton:This compiles fine on its own but blows the variance check at the
registerJourneycall site —JourneyDefinition.start(state: TState, ...)putsTStatecontravariantly, andunknowncan't satisfy any concrete state. The compiler error (Type 'unknown' is not assignable to type 'YourJourneyState') points at thepersistencearg ofregisterJourney, not the persistence declaration itself, so the actual fix (don't widenTStatetounknown; bind it per journey) is not obvious from the message.Real-world case: hit this in
ava-advisor-connect2wiring PSR + SARP through a sharedsessionStorageadapter. The team's first instinct was to silence the error withregisterJourney(journey as never, { persistence }), which masks the symptom on the wrong side and leaves persistence'sload(...) => SerializedJourney<unknown>lying to consumers. The right fix — a generic factory or distinct typed adapters per journey — was learnable fromexamples/react-router/journey-invokebut not flagged anywhere as the recommended shape for multi-journey shells.What the PR adds
Three explicit subheadings under the new "Registering multiple journeys" section:
examples/*/journey-invoke/shell/src/persistence.tsas the canonical demonstration.makeJourneyPersistence<TInput, TState>()invoked perregisterJourneycall. The shape used by adopters who want a single source of truth for SSR guards /keyFor/storageselection without per-journey repetition.<unknown, TInput>. Explains the variance failure and points back to the two safe shapes.No code changes — none of the existing examples or README snippets land on the anti-pattern, so this is purely additive documentation.
Test plan
makeJourneyPersistencecode block type-checks against the current public API surface (no missing imports).examples/*/journey-invokestill describes those files accurately.🤖 Generated with Claude Code
Summary by CodeRabbit