-
Notifications
You must be signed in to change notification settings - Fork 21
Add AddingMobxStateTree recipe #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add AddingMobxStateTree recipe #197
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall the new MST recipe is well-structured and technically sound, but there are a few maintainability concerns around persistence versioning, hook dependencies, and error handling that should be clarified so users don’t copy brittle patterns into production apps. The ROOT_STATE_STORAGE_KEY behavior, the callback dependency in useInitialRootStore, and the lack of a visible error path for failed rehydration are the most impactful areas to tighten up. Reactotron-related examples also rely on untyped globals and assume all plugin events have names; a short clarification or minor defensive coding would improve robustness. None of these are blockers, but addressing them would make the documentation safer and more future-proof for consumers.
Additional notes (3)
-
Maintainability |
docs/recipes/AddingMobxStateTree.md:253-256
The example Reactotron MST integration relies on(console as any).tron.trackMstNode(store), which is a deliberately untyped escape hatch. While this is fairly common in Reactotron usage, the recipe doesn’t call out that this is intentionally usinganyand depends on the globalconsole.tronside channel; readers might copy it into stricter codebases without understanding the trade-offs or that they should guard its usage to dev-only and Reactotron-present environments. -
Maintainability |
docs/recipes/AddingMobxStateTree.md:345-348
The Reactotronmstplugin filter uses a bare regex test onevent.namewithout any null/undefined guarding. In practice the plugin is unlikely to emit events without aname, but this is public recipe code that people may copy into customized setups; a defensive check (or at least a note) would prevent potential runtime errors if the event shape changes or if they pipe in custom events without aname. -
Readability |
docs/recipes/AddingMobxStateTree.md:335-349
The regex/postProcessSnapshot|@APPLY_SNAPSHOT/relies on operator precedence: as written it will be interpreted as/(postProcessSnapshot)|(@APPLY_SNAPSHOT)/, but that intent is not obvious at a glance and could be mis-edited by readers copying the pattern. More importantly, the filter ignores events by returningfalseonly when the test matches; this is a slightly inverted mental model for most people reading the example (filteroften suggests which events to keep).
For documentation code meant to be followed by a wide audience, it would be clearer to either (a) make the grouping in the regex explicit, or (b) invert the predicate to a positive include condition with a more self-documenting name, so people don't accidentally broaden or narrow the filter when customizing it.
Summary of changes
Summary of Changes
New recipe: AddingMobxStateTree.md
- Adds a detailed, step-by-step recipe for integrating MobX-State-Tree (MST) into a modern Ignite 11+ app that starts without MST.
- Covers installing MST and
reactotron-mst, creatingAuthenticationStore,CounterStore, andRootStoremodels, and persisting the root store viautils/storage. - Introduces
setupRootStore,useStores, anduseInitialRootStorehelpers, including a React Context–basedRootStoreProviderand dev-only Reactotron MST integration. - Documents wiring MST into
ReactotronConfig.tsand an example usage ofuseStores()fromWelcomeScreen.
Updated recipe: RemoveMobxStateTree.md
- Clarifies historical vs current default state management (MST vs React Context) in Ignite.
- Explicitly scopes the removal recipe to projects that already have MST wired in (e.g., pre–Ignite 11 or those that followed the new "Adding MST" recipe).
- Adds an
:::infocallout with guidance and cross-links to the new "Adding MobX-State-Tree to your Ignite app" recipe. - Minor wording and formatting improvements to better describe the goal of reaching a "blank slate" without global state management.
| import type { RootStore, RootStoreSnapshot } from "../RootStore" | ||
| import * as storage from "@/utils/storage" | ||
|
|
||
| const ROOT_STATE_STORAGE_KEY = "root-v1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ROOT_STATE_STORAGE_KEY is hard-coded to "root-v1" without any mention in the docs about versioning strategy or migration. In real-world apps, MST model evolution often requires bumping this key or adding migration logic; otherwise, older snapshots can cause subtle runtime issues when adding/removing model fields or changing types. Given this is a recipe intended as guidance, calling out how/when to change this key (or how to introduce migrations) would prevent people from copying this verbatim and later running into snapshot incompatibilities.
Suggestion
Consider adding a short note near the ROOT_STATE_STORAGE_KEY definition explaining that this acts as a simple version and should be bumped when making breaking changes to the shape of RootStoreSnapshot (or linking to a more advanced migration pattern, if one exists elsewhere in the docs). For example, you might add an inline comment or a short paragraph after this snippet describing how to handle snapshot incompatibilities over time. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this documentation tweak.
| export function useInitialRootStore(callback?: () => void | Promise<void>) { | ||
| const store = useStores() | ||
| const [rehydrated, setRehydrated] = useState(false) | ||
|
|
||
| useEffect(() => { | ||
| let unsubscribe: (() => void) | undefined | ||
|
|
||
| ;(async () => { | ||
| const { unsubscribe: setupUnsubscribe } = await setupRootStore(store) | ||
| unsubscribe = setupUnsubscribe | ||
|
|
||
| // Reactotron + MST integration (DEV only) | ||
| if (__DEV__ && (console as any).tron?.trackMstNode) { | ||
| ;(console as any).tron.trackMstNode(store) | ||
| } | ||
|
|
||
| setRehydrated(true) | ||
|
|
||
| if (callback) { | ||
| await callback() | ||
| } | ||
| })() | ||
|
|
||
| return () => { | ||
| unsubscribe?.() | ||
| } | ||
| }, [callback, store]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useInitialRootStore hook useEffect depends on callback, which is allowed to be an async function and is awaited inside the effect. If a parent passes an inline async arrow function, this will change on every render, retriggering the effect, re-running setupRootStore, reattaching the snapshot listener, and re-running the Reactotron wiring unnecessarily. In the worst case, this can lead to repeated rehydration cycles and multiple MST tracking setups unless the caller manually memoizes callback, which the recipe doesn’t warn about.
Suggestion
To make the example more robust, either (a) explicitly document that any callback passed into useInitialRootStore should be stable (e.g., wrapped in useCallback), or (b) remove callback from the dependency array and note that it’s only meant as a "run once after hydration" hook. The simplest fix for the recipe is probably to keep callback optional but omit it from the dependencies and describe this behavior in the text so users don’t accidentally create infinite re-init loops. Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating the snippet and surrounding explanation accordingly.
| ```ts title="/app/models/helpers/setupRootStore.ts" | ||
| import { applySnapshot, IDisposer, onSnapshot } from "mobx-state-tree" | ||
|
|
||
| import type { RootStore, RootStoreSnapshot } from "../RootStore" | ||
| import * as storage from "@/utils/storage" | ||
|
|
||
| const ROOT_STATE_STORAGE_KEY = "root-v1" | ||
|
|
||
| let disposer: IDisposer | undefined | ||
|
|
||
| export async function setupRootStore(rootStore: RootStore) { | ||
| let restoredState: RootStoreSnapshot | undefined | null | ||
|
|
||
| try { | ||
| // MMKV-backed helper that reads and parses JSON | ||
| restoredState = storage.load<RootStoreSnapshot>(ROOT_STATE_STORAGE_KEY) | ||
| if (restoredState) { | ||
| applySnapshot(rootStore, restoredState) | ||
| } | ||
| } catch (error) { | ||
| if (__DEV__ && error instanceof Error) { | ||
| console.error("Failed to load root store", error.message) | ||
| } | ||
| } | ||
|
|
||
| // stop tracking if we've already set up listeners | ||
| if (disposer) disposer() | ||
|
|
||
| // persist every snapshot to storage | ||
| disposer = onSnapshot(rootStore, (snapshot) => { | ||
| storage.save(ROOT_STATE_STORAGE_KEY, snapshot) | ||
| }) | ||
|
|
||
| const unsubscribe = () => { | ||
| disposer?.() | ||
| disposer = undefined | ||
| } | ||
|
|
||
| return { rootStore, restoredState, unsubscribe } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this example the setupRootStore function is declared async but only calls storage.load, which the comment describes as a synchronous "MMKV-backed helper that reads and parses JSON". Returning a Promise here adds asynchronous complexity without clear benefit and forces consumers (like useInitialRootStore) to spin up an IIFE and await the result even though the work is effectively synchronous. That can encourage cargo-culted async patterns when following the recipe and slightly complicates reasoning about store initialization.
Since the helper API is synchronous, consider making setupRootStore non-async and returning its result directly. If, in another app, storage.load is asynchronous, that can be documented as an alternative, but the core recipe should reflect the actual behavior of the Ignite boilerplate utilities.
Suggestion
You can simplify the example by making setupRootStore synchronous and adjusting the hook accordingly, which also makes the control flow easier for readers to follow:
// app/models/helpers/setupRootStore.ts
import { applySnapshot, IDisposer, onSnapshot } from "mobx-state-tree"
import type { RootStore, RootStoreSnapshot } from "../RootStore"
import * as storage from "@/utils/storage"
const ROOT_STATE_STORAGE_KEY = "root-v1"
let disposer: IDisposer | undefined
export function setupRootStore(rootStore: RootStore) {
let restoredState: RootStoreSnapshot | undefined | null
try {
restoredState = storage.load<RootStoreSnapshot>(ROOT_STATE_STORAGE_KEY)
if (restoredState) {
applySnapshot(rootStore, restoredState)
}
} catch (error) {
if (__DEV__ && error instanceof Error) {
console.error("Failed to load root store", error.message)
}
}
if (disposer) disposer()
disposer = onSnapshot(rootStore, (snapshot) => {
storage.save(ROOT_STATE_STORAGE_KEY, snapshot)
})
const unsubscribe = () => {
disposer?.()
disposer = undefined
}
return { rootStore, restoredState, unsubscribe }
}Then in useInitialRootStore you can remove the async IIFE:
useEffect(() => {
const { unsubscribe } = setupRootStore(store)
if (__DEV__ && (console as any).tron?.trackMstNode) {
;(console as any).tron.trackMstNode(store)
}
setRehydrated(true)
if (callback) {
void callback()
}
return () => {
unsubscribe()
}
}, [callback, store])This keeps the example aligned with the MMKV-backed storage helpers while being easier to reason about. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| ```ts title="/app/models/helpers/useStores.ts" | ||
| import { createContext, useContext, useEffect, useState } from "react" | ||
|
|
||
| import type { RootStore } from "../RootStore" | ||
| import { RootStoreModel } from "../RootStore" | ||
| import { setupRootStore } from "./setupRootStore" | ||
|
|
||
| // Create the singleton RootStore instance | ||
| const rootStore = RootStoreModel.create({}) | ||
|
|
||
| const RootStoreContext = createContext<RootStore>(rootStore) | ||
|
|
||
| export const RootStoreProvider = RootStoreContext.Provider | ||
|
|
||
| export const useStores = () => useContext(RootStoreContext) | ||
|
|
||
| export function useInitialRootStore(callback?: () => void | Promise<void>) { | ||
| const store = useStores() | ||
| const [rehydrated, setRehydrated] = useState(false) | ||
|
|
||
| useEffect(() => { | ||
| let unsubscribe: (() => void) | undefined | ||
|
|
||
| ;(async () => { | ||
| const { unsubscribe: setupUnsubscribe } = await setupRootStore(store) | ||
| unsubscribe = setupUnsubscribe | ||
|
|
||
| // Reactotron + MST integration (DEV only) | ||
| if (__DEV__ && (console as any).tron?.trackMstNode) { | ||
| ;(console as any).tron.trackMstNode(store) | ||
| } | ||
|
|
||
| setRehydrated(true) | ||
|
|
||
| if (callback) { | ||
| await callback() | ||
| } | ||
| })() | ||
|
|
||
| return () => { | ||
| unsubscribe?.() | ||
| } | ||
| }, [callback, store]) | ||
|
|
||
| return { rootStore: store, rehydrated } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useInitialRootStore both depends on useStores() (which reads from RootStoreContext) and is intended to be called from app/app.tsx before the rest of the UI. The recipe, however, never shows wrapping the app in RootStoreProvider, so as written a reader could easily end up calling useInitialRootStore without any provider in place and still not notice because RootStoreContext is initialized with the global rootStore default value. This hides a missing provider error and encourages usage patterns that are easy to mis-apply in a real app (e.g., trying to mount the provider after running the hook).
Given that this is instructional documentation, it would be clearer and safer to show (a) wrapping the app tree in RootStoreProvider at the top level and (b) making RootStoreContext default to undefined so missing providers are caught early, with the hook throwing a meaningful error. That would help prevent subtle bugs when people adapt this pattern.
Suggestion
To make provider usage explicit and fail-fast when forgotten, you can tweak the example along these lines:
// app/models/helpers/useStores.ts
import { createContext, useContext, useEffect, useState } from "react"
import type { RootStore } from "../RootStore"
import { RootStoreModel } from "../RootStore"
import { setupRootStore } from "./setupRootStore"
const rootStore = RootStoreModel.create({})
const RootStoreContext = createContext<RootStore | undefined>(undefined)
export const RootStoreProvider = RootStoreContext.Provider
export function useStores(): RootStore {
const context = useContext(RootStoreContext)
if (!context) {
throw new Error("useStores must be used within a RootStoreProvider")
}
return context
}
export function useInitialRootStore(callback?: () => void | Promise<void>) {
const store = useStores()
const [rehydrated, setRehydrated] = useState(false)
useEffect(() => {
const { unsubscribe } = setupRootStore(store)
if (__DEV__ && (console as any).tron?.trackMstNode) {
;(console as any).tron.trackMstNode(store)
}
setRehydrated(true)
if (callback) {
void callback()
}
return () => {
unsubscribe()
}
}, [callback, store])
return { rootStore: store, rehydrated }
}And in the app/app.tsx section of the recipe, explicitly show wrapping your navigation tree (or entire app) with RootStoreProvider, passing the singleton rootStore:
import { RootStoreProvider, useInitialRootStore } from "./models"
export function App() {
const { rehydrated, rootStore } = useInitialRootStore()
if (!rehydrated || /* existing guards */) return null
return (
<RootStoreProvider value={rootStore}>
{/* existing providers and navigation */}
</RootStoreProvider>
)
}This makes the context wiring obvious and avoids silently falling back to a default root store when the provider is missing. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
Adds a new recipe for adding MobX-State-Tree back into a modern Ignite app and clarifies that the existing removal recipe is primarily for older MST-based projects.
Changes
docs/recipes/AddingMobxStateTree.mddocumenting how to:AuthenticationStore,CounterStore, andRootStoreutils/storagehelpersuseStores()/useInitialRootStore()and wire them intoapp/app.tsxReactotronConfig.tsand consume a store fromWelcomeScreendocs/recipes/RemoveMobxStateTree.mdto:Verification
yarn typecheck(root) — passedyarn build(root) — passed; Docusaurus built the site successfullyRefs #196.