Clarify handling of async task revs

src/components/ComposerV2/store/index.ts

@@ -16,6 +16,7 @@ import {buildPostMediaItem} from '#/components/ComposerV2/store/utils/buildPostM
 import {buildThreadPost} from '#/components/ComposerV2/store/utils/buildThreadPost'
 import {classifyUriTarget} from '#/components/ComposerV2/store/utils/classifyUriTarget'
 import {computePostMediaSelectionsRemaining} from '#/components/ComposerV2/store/utils/computePostMediaSelectionsRemaining'
+import {createAsyncTaskRev} from '#/components/ComposerV2/store/utils/createAsyncTaskRev'
 import {filterMediaInputs} from '#/components/ComposerV2/store/utils/filterMediaInputs'
 
 type Listener = () => void
@@ -47,20 +48,14 @@ export function createThreadStore(options: {
   const uploadTasks = new Map<string, UploadTask>()
 
   /**
-   * Per-slot generation counters. Quote and embed are orthogonal slots, so
+   * Per-slot revision counters. Quote and embed are orthogonal slots, so
-   * each has its own counter; cancelling one doesn't invalidate the other.
+   * each has its own counter; invalidating one doesn't invalidate the
-   * Every action that starts or invalidates a resolution for a slot bumps
+   * other. Every action that starts or supersedes a resolution for a slot
-   * that slot's gen, and the worker callback compares its captured gen
+   * calls `incrementFor(postId)`, and the worker callback closes over the
-   * against the current value before writing to state.
+   * returned `isCurrent` checker to decide whether to write back.
    */
-  const quoteGenByPost = new Map<string, number>()
+  const quoteRev = createAsyncTaskRev()
-  const embedGenByPost = new Map<string, number>()
+  const embedRev = createAsyncTaskRev()
-
-  function bumpGen(map: Map<string, number>, postId: string): number {
-    const next = (map.get(postId) ?? 0) + 1
-    map.set(postId, next)
-    return next
-  }
 
   /**
    * Action bodies mutate `s` in place. Returning `null` signals a no-op (the
@@ -140,12 +135,10 @@ export function createThreadStore(options: {
       if (!(postId in s.posts)) return null
       // Cancel any in-flight uploads for media on this post before dropping it.
       for (const m of s.posts[postId].media) cancelUploadTask(m.id)
-      // Bump (and drop) both gens so any stale resolution callbacks for
+      // Drop both rev entries so any stale resolution callbacks for this
-      // this post can never write back into state.
+      // post can never write back into state.
-      bumpGen(quoteGenByPost, postId)
+      quoteRev.clearFor(postId)
-      bumpGen(embedGenByPost, postId)
+      embedRev.clearFor(postId)
-      quoteGenByPost.delete(postId)
-      embedGenByPost.delete(postId)
       delete s.posts[postId]
       s.isDirty = true
       return s
@@ -305,7 +298,7 @@ export function createThreadStore(options: {
    *
    * Otherwise, pending state is written to the target slot synchronously and
    * `resolveLink` runs in the background. The outcome lands in the same slot
-   * (resolved or failed). Cancellation is per-slot gen-based.
+   * (resolved or failed). Cancellation is per-slot rev-based.
    */
   function addUri(postId: string, uri: string) {
     const post = state.posts[postId]
@@ -317,7 +310,7 @@ export function createThreadStore(options: {
       // No-op only when the slot has a settled value. Pending and failed
       // states are replaceable (failed.retry() relies on this).
       if (post.quote?.state === 'resolved') return
-      const gen = bumpGen(quoteGenByPost, postId)
+      const rev = quoteRev.incrementFor(postId)
       mutateState(s => {
         const p = s.posts[postId]
         if (!p) return null
@@ -329,7 +322,7 @@ export function createThreadStore(options: {
         postId,
         uri,
         resolveLink: resolveLinkOverride,
-        onResolve: handleQuoteResolution(gen, uri),
+        onResolve: handleQuoteResolution(rev, uri),
       })
       return
     }
@@ -344,7 +337,7 @@ export function createThreadStore(options: {
     if (embedSettled) return
     if (post.media.length > 0) return
 
-    const gen = bumpGen(embedGenByPost, postId)
+    const rev = embedRev.incrementFor(postId)
     mutateState(s => {
       const p = s.posts[postId]
       if (!p) return null
@@ -356,14 +349,14 @@ export function createThreadStore(options: {
       postId,
       uri,
       resolveLink: resolveLinkOverride,
-      onResolve: handleEmbedResolution(gen, uri),
+      onResolve: handleEmbedResolution(rev, uri),
     })
   }
 
-  function handleQuoteResolution(gen: number, uri: string) {
+  function handleQuoteResolution(rev: number, uri: string) {
     return (postId: string, outcome: LinkResolutionOutcome) => {
       if (destroyed) return
-      if (quoteGenByPost.get(postId) !== gen) return
+      if (!quoteRev.isCurrentFor(postId, rev)) return
 
       // Pre-classification said this was a post URL. If resolveLink disagrees
       // (deleted post, embedding disabled, network error, etc.), surface as
@@ -409,10 +402,10 @@ export function createThreadStore(options: {
     }
   }
 
-  function handleEmbedResolution(gen: number, uri: string) {
+  function handleEmbedResolution(rev: number, uri: string) {
     return (postId: string, outcome: LinkResolutionOutcome) => {
       if (destroyed) return
-      if (embedGenByPost.get(postId) !== gen) return
+      if (!embedRev.isCurrentFor(postId, rev)) return
 
       // Pre-classification said this was a non-post URL. If resolveLink
       // surprises us with a post outcome, treat as failure rather than
@@ -455,7 +448,7 @@ export function createThreadStore(options: {
   }
 
   function removeEmbed(postId: string) {
-    bumpGen(embedGenByPost, postId)
+    embedRev.incrementFor(postId)
     mutateState(s => {
       const post = s.posts[postId]
       if (!post) return null
@@ -469,13 +462,13 @@ export function createThreadStore(options: {
   /**
    * Direct setter for an already-resolved quote. Used for draft restore and
    * any UI flow that already has the post ref+view in hand. Bumps the quote
-   * gen so any in-flight resolution is invalidated.
+   * rev so any in-flight resolution is invalidated.
    */
   function setQuoteEmbed(
     postId: string,
     ref: {uri: string; cid: string; view?: AppBskyFeedDefs.PostView},
   ) {
-    bumpGen(quoteGenByPost, postId)
+    quoteRev.incrementFor(postId)
     mutateState(s => {
       const post = s.posts[postId]
       if (!post) return null
@@ -491,7 +484,7 @@ export function createThreadStore(options: {
   }
 
   function removeQuoteEmbed(postId: string) {
-    bumpGen(quoteGenByPost, postId)
+    quoteRev.incrementFor(postId)
     mutateState(s => {
       const post = s.posts[postId]
       if (!post) return null
@@ -610,8 +603,8 @@ export function createThreadStore(options: {
       destroyed = true
       for (const task of uploadTasks.values()) task.cancel()
       uploadTasks.clear()
-      quoteGenByPost.clear()
+      quoteRev.clearAll()
-      embedGenByPost.clear()
+      embedRev.clearAll()
     },
     getState() {
       return state

src/components/ComposerV2/store/utils/createAsyncTaskRev.ts

@@ -0,0 +1,48 @@
+/**
+ * A keyed revision counter used to cancel stale async work. Callers
+ * `incrementFor(key)` to bump a key's revision and capture the new value,
+ * then later `isCurrentFor(key, rev)` to check whether their captured
+ * revision is still the live one. If it isn't, a newer caller has
+ * superseded them and they should bail.
+ *
+ * Example:
+ *   const rev = task.incrementFor(postId)
+ *   doAsyncWork().then(result => {
+ *     if (!task.isCurrentFor(postId, rev)) return
+ *     // ... write result to state
+ *   })
+ *
+ * `clearFor(key)` drops the key - any later isCurrentFor call for that
+ * key returns false. Right hook to call from removePost.
+ * `clearAll()` drops every key - used on store destroy.
+ */
+export type AsyncTaskRev = {
+  /** Bump the key's revision and return the new value. */
+  incrementFor(key: string): number
+  /** True while `rev` is still the current revision for `key`. */
+  isCurrentFor(key: string, rev: number): boolean
+  /** Drop the key. Future isCurrentFor calls for this key return false. */
+  clearFor(key: string): void
+  /** Drop every key. Future isCurrentFor calls return false. */
+  clearAll(): void
+}
+
+export function createAsyncTaskRev(): AsyncTaskRev {
+  const counters = new Map<string, number>()
+  return {
+    incrementFor(key) {
+      const next = (counters.get(key) ?? 0) + 1
+      counters.set(key, next)
+      return next
+    },
+    isCurrentFor(key, rev) {
+      return counters.get(key) === rev
+    },
+    clearFor(key) {
+      counters.delete(key)
+    },
+    clearAll() {
+      counters.clear()
+    },
+  }
+}