Skip to content

Conversation

@charliecreates
Copy link

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

  • Added docs/recipes/AddingMobxStateTree.md documenting how to:
    • Install MST + Reactotron integration
    • Create a simple AuthenticationStore, CounterStore, and RootStore
    • Persist the root store using the shared utils/storage helpers
    • Expose useStores() / useInitialRootStore() and wire them into app/app.tsx
    • Optionally hook MST into ReactotronConfig.ts and consume a store from WelcomeScreen
  • Updated docs/recipes/RemoveMobxStateTree.md to:
    • Note that Ignite 11+ uses React Context by default
    • Link to the Ignite 11 release and blog post
    • Clearly scope the recipe to projects that already have MST wired in
    • Cross-link to the new Adding MobX-State-Tree to your Ignite app recipe

Verification

# TypeScript typecheck
yarn typecheck

# Docusaurus production build
yarn build
  • yarn typecheck (root) — passed
  • yarn build (root) — passed; Docusaurus built the site successfully

Refs #196.

Copy link
Author

@charliecreates charliecreates bot left a 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 using any and depends on the global console.tron side 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 Reactotron mst plugin filter uses a bare regex test on event.name without any null/undefined guarding. In practice the plugin is unlikely to emit events without a name, 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 a name.

  • 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 returning false only when the test matches; this is a slightly inverted mental model for most people reading the example (filter often 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, creating AuthenticationStore, CounterStore, and RootStore models, and persisting the root store via utils/storage.
  • Introduces setupRootStore, useStores, and useInitialRootStore helpers, including a React Context–based RootStoreProvider and dev-only Reactotron MST integration.
  • Documents wiring MST into ReactotronConfig.ts and an example usage of useStores() from WelcomeScreen.

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 :::info callout 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"
Copy link
Author

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.

Comment on lines +242 to +268
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])
Copy link
Author

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.

Comment on lines +173 to +212
```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 }
}
Copy link
Author

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.

Comment on lines +226 to +271
```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 }
}
Copy link
Author

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.

@charliecreates charliecreates bot removed the request for review from CharlieHelps November 21, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants