Skip to content

fix: useeffect should not return subscription object #416

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.

Sign up for GitHub

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

Open
vilindh wants to merge 1 commit into
base: main
Choose a base branch
from
+1 −1
Open

fix: useeffect should not return subscription object #416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/create/src/frameworks/react/examples/events/assets/src/components/RemyAssistant.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export default function RemyAssistant({

// Sync with store for header control
useEffect(() => {
return showRemyAssistant.subscribe(() => {
showRemyAssistant.subscribe(() => {
setIsOpen(showRemyAssistant.state)
})
}, [])
Comment on lines 93 to 97
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Compare effect patterns in related components.
rg -n --type=tsx -C2 'show(Remy|Resume)Assistant\.subscribe' packages/create/src/frameworks/react/examples

# 2) Find the `@tanstack/store` version used by the repo.
dep=$(rg -n --no-heading '"@tanstack/store"\s*:\s*"[^"]+"' --glob '**/package.json' | head -n1 | sed -E 's/.*"@tanstack\/store"\s*:\s*"([^"]+)".*/\1/')
ver=$(printf '%s' "$dep" | sed -E 's/^[^0-9]*//')
echo "Detected `@tanstack/store` range: $dep (normalized: $ver)"

# 3) Inspect published typings/source for subscribe signature in that version.
for url in \
  "https://unpkg.com/@tanstack/store@${ver}/dist/esm/index.d.ts" \
  "https://unpkg.com/@tanstack/store@${ver}/build/lib/index.d.ts" \
  "https://unpkg.com/@tanstack/store@${ver}/src/index.ts"
do
  echo "Checking $url"
  if curl -fsSL "$url" | rg -n 'subscribe\s*\(' -C2; then
    echo "Signature found in $url"
    break
  fi
done

Repository: TanStack/cli

Length of output: 83

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Fix: Use proper glob pattern instead of unrecognized --type=tsx
echo "=== Searching for subscription patterns in React example components ==="
rg -n -C2 'show(Remy|Resume)Assistant\.subscribe' packages/create/src/frameworks/react/examples --type-list | head -5 || true
rg -n -C2 'show(Remy|Resume)Assistant\.subscribe' packages/create/src/frameworks/react/examples -g '*.tsx' 2>/dev/null || echo "Pattern not found with -g flag, trying without type restriction"
rg -n -C2 'show(Remy|Resume)Assistant\.subscribe' packages/create/src/frameworks/react/examples || echo "No matches found"

Repository: TanStack/cli

Length of output: 3098

🏁 Script executed:

#!/bin/bash
# Read the target file to see the actual code at lines 93-97
echo "=== RemyAssistant.tsx (lines 85-105) ==="
cat -n packages/create/src/frameworks/react/examples/events/assets/src/components/RemyAssistant.tsx | sed -n '85,105p'

Repository: TanStack/cli

Length of output: 795

🏁 Script executed:

#!/bin/bash
# Read the comparison file mentioned in the review
echo "=== ResumeAssistant.tsx (lines 75-95) ==="
cat -n packages/create/src/frameworks/react/examples/resume/assets/src/components/ResumeAssistant.tsx | sed -n '75,95p'

Repository: TanStack/cli

Length of output: 839

🏁 Script executed:

#!/bin/bash
# Find the `@tanstack/store` version in package.json
echo "=== `@tanstack/store` version ==="
rg '"@tanstack/store"' -g '**/package.json' -A1 packages/create

Repository: TanStack/cli

Length of output: 839

🏁 Script executed:

#!/bin/bash
# Search for `@tanstack/store` documentation or type definitions in the repo
echo "=== Checking if `@tanstack/store` types are available in node_modules or lockfile ==="
find . -name "package-lock.json" -o -name "yarn.lock" -o -name "pnpm-lock.yaml" | head -3

# Check if there are any existing imports or usage patterns in the repo
echo "=== Checking other Store.subscribe usages in codebase ==="
rg 'subscribe\s*\(' packages/ --max-count=10 -B2 -A2 || echo "No other patterns found"

Repository: TanStack/cli

Length of output: 4547

🌐 Web query:

@tanstack/store subscribe method return type API documentation

💡 Result:

The subscribe method on Store instances from @tanstack/store returns a Subscription object. This object has an unsubscribe method: unsubscribe: => void. Signature: subscribe(observerOrFn: Observer | (value: T) => void): Subscription Subscription interface: interface Subscription { unsubscribe: => void; } Examples from docs: const subscription = store.subscribe((value) => { ... }); subscription.unsubscribe; This is consistent across Store, ReadonlyStore, Atom, etc. as they extend Subscribable.

Citations:

Return the unsubscribe function to ensure effect cleanup on unmount.

Line 94 subscribes to the store without returning the cleanup function. This causes listeners to persist after unmount, leading to memory leaks and duplicate subscriptions on remounts. Compare with ResumeAssistant.tsx:84 which correctly returns the subscription.

Proposed fix 
   // Sync with store for header control
   useEffect(() => {
-    showRemyAssistant.subscribe(() => {
+    const unsubscribe = showRemyAssistant.subscribe(() => {
       setIsOpen(showRemyAssistant.state)
     })
+    return unsubscribe
   }, [])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
return showRemyAssistant.subscribe(() => {
showRemyAssistant.subscribe(() => {
setIsOpen(showRemyAssistant.state)
})
}, [])
useEffect(() => {
const unsubscribe = showRemyAssistant.subscribe(() => {
setIsOpen(showRemyAssistant.state)
})
return unsubscribe
}, [])
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@packages/create/src/frameworks/react/examples/events/assets/src/components/RemyAssistant.tsx`
around lines 93 - 97, The useEffect in RemyAssistant subscribes to
showRemyAssistant but does not return the unsubscribe function, causing
lingering listeners; update the effect that calls
showRemyAssistant.subscribe(...) to capture the returned unsubscribe (or
subscription) and return it from the useEffect cleanup so the listener is
removed on unmount (match the pattern used in ResumeAssistant.tsx around its
subscription); ensure you still call setIsOpen(showRemyAssistant.state) on
subscription events and return the unsubscribe function from useEffect.

Expand Down