fix: useeffect should not return subscription object#416
fix: useeffect should not return subscription object#416vilindh wants to merge 1 commit intoTanStack:mainfrom
Conversation
📝 WalkthroughWalkthroughRemoved a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/create/src/frameworks/react/examples/events/assets/src/components/RemyAssistant.tsx`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 096d44e8-5af6-47dd-b732-afcbab5a1198
📒 Files selected for processing (1)
packages/create/src/frameworks/react/examples/events/assets/src/components/RemyAssistant.tsx
| useEffect(() => { | ||
| return showRemyAssistant.subscribe(() => { | ||
| showRemyAssistant.subscribe(() => { | ||
| setIsOpen(showRemyAssistant.state) | ||
| }) | ||
| }, []) |
There was a problem hiding this comment.
🧩 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
doneRepository: 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/createRepository: 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:
- 1: https://tanstack.com/store/latest/docs/reference/interfaces/Subscribable
- 2: https://tanstack.com/store/v0/docs/reference/classes/Store
- 3: https://tanstack.com/store/latest/docs/reference/interfaces/Subscription
- 4: https://www.mintlify.com/TanStack/store/concepts/subscriptions
- 5: https://www.mintlify.com/TanStack/store/api/core/create-store
- 6: https://tanstack.com/store/latest/docs/reference/classes/Store
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.
| 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.
1 participant
This PR fixes an issue with useEffect hook faulty returning a subscription object;
useEffect should not return, unless a cleanup function.
Current implication of return is that navigating to "Meet Our Speakers" from the tryout example starting page, results
in a crash-landing.
Summary by CodeRabbit