fix(#26): preserve un-synced localState across subscription rebuild

src/collection.ts

@@ -73,6 +73,13 @@ export interface CollectionOptions<TData extends FirestoreObject> {
     collectionPath?: string
     /** Override read-only setting */
     readOnly?: boolean
+    /**
+     * Dynamic read-only getter. When provided, evaluated on each mutation
+     * call instead of the static `readOnly` option. The hook layer uses this
+     * so that toggling `readOnly` at runtime does not tear down the
+     * subscription or discard pending local state.
+     */
+    getReadOnly?: () => boolean | undefined
     /** Additional query constraints */
     queryConstraints?: QueryConstraint[]
     /** Callback for pushing undo actions */
@@ -137,7 +144,7 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     /** Force sync now */
     sync: () => Promise<void>
 } => {
-    const { store, definition, collectionPath: resolvedPath, readOnly, queryConstraints: extraConstraints, onPushUndo } = options
+    const { store, definition, collectionPath: resolvedPath, readOnly, getReadOnly, queryConstraints: extraConstraints, onPushUndo } = options
     const { firestore, autosave: defaultAutosave, minLoadTime: defaultMinLoadTime } = store
 
     const {
@@ -152,7 +159,12 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
         schema,
     } = definition
 
-    const isReadOnly = readOnly ?? definitionReadOnly ?? false
+    // When the hook layer supplies getReadOnly, evaluate it on each mutation
+    // call so that toggling readOnly at runtime takes effect without rebuilding
+    // the subscription. Direct API callers that pass the static `readOnly`
+    // option work as before.
+    const resolveReadOnly = () =>
+        (getReadOnly ? getReadOnly() : readOnly) ?? definitionReadOnly ?? false
     // Prefer the caller-resolved path. Fall back to a string `definition.path`
     // for ergonomic direct use; if both are missing, the caller forgot to
     // resolve a function path.
@@ -184,6 +196,9 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     let retryTimeout: ReturnType<typeof setTimeout> | null = null
     let minLoadTimeElapsed = false
     let loaded = false
+    // Set to true by stop() so that an in-flight fire-and-forget sync()
+    // completing after teardown doesn't re-register a stale sync key.
+    let stopped = false
     // Cached handle — returns the same reference until notify() invalidates
     // it. Lets useSyncExternalStore consumers rely on handle identity.
     let cachedHandle: CollectionHandle<TData> | null = null
@@ -215,7 +230,9 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
         cachedHandle = null
         const publicState = getPublicState()
         subscribers.forEach((fn) => fn(publicState))
-        store.reportSyncState(syncKey, publicState.isSynced)
+        if (!stopped) {
+            store.reportSyncState(syncKey, publicState.isSynced)
+        }
     }
 
     // Pre-snapshot mutations are unsafe because computing a partial-update
@@ -235,7 +252,7 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
         diff: WithFieldValue<DeepPartial<Record<string, TData>>>,
         undoOptions: UpdateOptions = {}
     ) => {
-        if (isReadOnly) return
+        if (resolveReadOnly()) return
         if (state.syncState === undefined) {
             warnNoSnapshot('update')
             return
@@ -307,7 +324,7 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
             ? maybeUndoOptions
             : (dataOrOptions as UpdateOptions | undefined)) ?? {}
 
-        if (isReadOnly) return undefined
+        if (resolveReadOnly()) return undefined
         if (state.syncState === undefined) {
             // Bail rather than queueing: an explicit id that happens to exist
             // on the server would round-trip through computeDiff and clobber
@@ -360,7 +377,7 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     }
 
     const removeDocument = (id: string, undoOptions: UpdateOptions = {}) => {
-        if (isReadOnly) return
+        if (resolveReadOnly()) return
         if (state.syncState === undefined) {
             warnNoSnapshot('remove')
             return
@@ -556,6 +573,10 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     const startListener = () => {
         if (unsubscribeListener) return
 
+        // Clear the stopped flag so notify() resumes reporting sync state.
+        // Needed for the retryOnError path: stop() sets stopped=true, then
+        // startListener() re-activates the subscription.
+        stopped = false
         loaded = false
         minLoadTimeElapsed = false
 
@@ -601,6 +622,9 @@ export const createCollectionSubscription = <TData extends FirestoreObject>(
     }
 
     const stop = () => {
+        // Prevent any in-flight fire-and-forget sync() from re-registering
+        // a stale sync key after teardown.
+        stopped = true
         if (retryTimeout) {
             clearTimeout(retryTimeout)
             retryTimeout = null

src/document.ts

@@ -54,6 +54,13 @@ export interface DocumentOptions<TData extends FirestoreObject> {
     collectionPath?: string
     /** Override read-only setting */
     readOnly?: boolean
+    /**
+     * Dynamic read-only getter. When provided, evaluated on each mutation
+     * call instead of the static `readOnly` option. The hook layer uses this
+     * so that toggling `readOnly` at runtime does not tear down the
+     * subscription or discard pending local state.
+     */
+    getReadOnly?: () => boolean | undefined
     /** Callback for pushing undo actions */
     onPushUndo?: (
         undoAction: () => void,
@@ -127,7 +134,7 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     /** Force sync now */
     sync: () => Promise<void>
 } => {
-    const { store, definition, docId, collectionPath: resolvedCollectionPath, readOnly, onPushUndo } = options
+    const { store, definition, docId, collectionPath: resolvedCollectionPath, readOnly, getReadOnly, onPushUndo } = options
     const { firestore, autosave: defaultAutosave, minLoadTime: defaultMinLoadTime } = store
 
     const {
@@ -141,7 +148,12 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
         schema,
     } = definition
 
-    const isReadOnly = readOnly ?? definitionReadOnly ?? false
+    // When the hook layer supplies getReadOnly, evaluate it on each mutation
+    // call so that toggling readOnly at runtime takes effect without rebuilding
+    // the subscription. Direct API callers that pass the static `readOnly`
+    // option work as before.
+    const resolveReadOnly = () =>
+        (getReadOnly ? getReadOnly() : readOnly) ?? definitionReadOnly ?? false
     // Prefer the caller-resolved docId. Fall back to a string `definition.id`
     // for ergonomic direct use; if both are missing, the caller forgot to
     // resolve a function id and we surface that immediately.
@@ -186,6 +198,9 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     let minLoadTimeout: ReturnType<typeof setTimeout> | null = null
     let minLoadTimeElapsed = false
     let loaded = false
+    // Set to true by stop() so that an in-flight fire-and-forget sync()
+    // completing after teardown doesn't re-register a stale sync key.
+    let stopped = false
     // Cached handle — returns the same reference until notify() invalidates
     // it. Lets useSyncExternalStore consumers rely on handle identity.
     let cachedHandle: DocumentHandle<TData> | null = null
@@ -223,14 +238,16 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
         cachedHandle = null
         const publicState = getPublicState()
         subscribers.forEach((fn) => fn(publicState))
-        store.reportSyncState(syncKey, publicState.isSynced)
+        if (!stopped) {
+            store.reportSyncState(syncKey, publicState.isSynced)
+        }
     }
 
     const updateState = (
         diff: WithFieldValue<DeepPartial<TData>>,
         undoOptions: UpdateOptions = {}
     ) => {
-        if (isReadOnly) return
+        if (resolveReadOnly()) return
 
         const currentData = getMergedData()
         if (!currentData) {
@@ -280,7 +297,7 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     }
 
     const setData = (data: TData, undoOptions: UpdateOptions = {}) => {
-        if (isReadOnly) return
+        if (resolveReadOnly()) return
 
         // Use schema.parse as a validation guard — throw on bad input — but
         // discard the parsed result and store the caller's original object.
@@ -328,7 +345,7 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     }
 
     const deleteDocument = (undoOptions: UpdateOptions = {}) => {
-        if (isReadOnly) return
+        if (resolveReadOnly()) return
 
         const currentData = getMergedData()
         // Nothing to delete — bail rather than queueing a no-op deleteDoc.
@@ -559,6 +576,10 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     const load = () => {
         if (unsubscribeListener) return
 
+        // Clear the stopped flag so notify() resumes reporting sync state.
+        // Needed for the retryOnError path: stop() sets stopped=true, then
+        // load() re-activates the subscription.
+        stopped = false
         loaded = false
         minLoadTimeElapsed = false
 
@@ -586,6 +607,9 @@ export const createDocumentSubscription = <TData extends FirestoreObject>(
     }
 
     const stop = () => {
+        // Prevent any in-flight fire-and-forget sync() from re-registering
+        // a stale sync key after teardown.
+        stopped = true
         if (unsubscribeListener) {
             unsubscribeListener()
             unsubscribeListener = null