mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
Fix useEffect anti-patterns across renderer (#9)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
abcdc6d451
commit
578cd02372
12 changed files with 1054 additions and 89 deletions
341
.agents/skills/react-useeffect/README.md
Normal file
341
.agents/skills/react-useeffect/README.md
Normal file
|
|
@ -0,0 +1,341 @@
|
|||
# React useEffect Best Practices
|
||||
|
||||
A comprehensive guide teaching when to use `useEffect` in React, and more importantly, when NOT to use it. This skill is based on official React documentation and provides practical alternatives to common useEffect anti-patterns.
|
||||
|
||||
## Purpose
|
||||
|
||||
Effects are an **escape hatch** from React's reactive paradigm. They let you synchronize with external systems like browser APIs, third-party widgets, or network requests. However, many developers overuse Effects for tasks that React handles better through other means.
|
||||
|
||||
This skill helps you:
|
||||
|
||||
- Identify when you truly need an Effect vs. when you don't
|
||||
- Recognize common anti-patterns and their fixes
|
||||
- Apply better alternatives like `useMemo`, `key` prop, and event handlers
|
||||
- Write Effects that are clean, maintainable, and free from race conditions
|
||||
|
||||
## When to Use This Skill
|
||||
|
||||
Use this skill when you're:
|
||||
|
||||
- Writing or reviewing `useEffect` code
|
||||
- Using `useState` to store derived values
|
||||
- Implementing data fetching or subscriptions
|
||||
- Synchronizing state between components
|
||||
- Facing bugs with stale data or race conditions
|
||||
- Wondering if your Effect is necessary
|
||||
|
||||
**Trigger phrases:**
|
||||
|
||||
- "Should I use useEffect for this?"
|
||||
- "How do I fix this useEffect?"
|
||||
- "My Effect is causing too many re-renders"
|
||||
- "Data fetching with useEffect"
|
||||
- "Reset state when props change"
|
||||
- "Derived state from props"
|
||||
|
||||
## How It Works
|
||||
|
||||
This skill provides guidance through three key resources:
|
||||
|
||||
1. **Quick Reference Table** - Fast lookup for common scenarios with DO/DON'T patterns
|
||||
2. **Decision Tree** - Visual flowchart to determine the right approach
|
||||
3. **Detailed Anti-Patterns** - 9 common mistakes with explanations and fixes
|
||||
4. **Better Alternatives** - 8 proven patterns to replace unnecessary Effects
|
||||
|
||||
The skill teaches you to ask the right questions:
|
||||
|
||||
- Is there an external system involved?
|
||||
- Am I responding to a user event or component appearance?
|
||||
- Can this value be calculated during render?
|
||||
- Do I need to reset state when a prop changes?
|
||||
|
||||
## Key Features
|
||||
|
||||
### 1. Quick Reference Guide
|
||||
|
||||
Visual table showing the DO/DON'T for common scenarios:
|
||||
|
||||
- Derived state from props/state
|
||||
- Expensive calculations
|
||||
- Resetting state on prop change
|
||||
- User event responses
|
||||
- Notifying parent components
|
||||
- Data fetching
|
||||
|
||||
### 2. Decision Tree
|
||||
|
||||
Clear flowchart that guides you from "Need to respond to something?" to the correct solution:
|
||||
|
||||
- User interaction → Event handler
|
||||
- Component appeared → Effect (for external sync/analytics)
|
||||
- Derived value needed → Calculate during render (+ useMemo if expensive)
|
||||
- Reset state on prop change → Key prop
|
||||
|
||||
### 3. Anti-Pattern Recognition
|
||||
|
||||
Detailed examples of 9 common mistakes:
|
||||
|
||||
1. Redundant state for derived values
|
||||
2. Filtering/transforming data in Effect
|
||||
3. Resetting state on prop change
|
||||
4. Event-specific logic in Effect
|
||||
5. Chains of Effects
|
||||
6. Notifying parent via Effect
|
||||
7. Passing data up to parent
|
||||
8. Fetching without cleanup (race conditions)
|
||||
9. App initialization in Effect
|
||||
|
||||
Each anti-pattern includes:
|
||||
|
||||
- Bad example with explanation
|
||||
- Good example with fix
|
||||
- Why the anti-pattern is problematic
|
||||
|
||||
### 4. Better Alternatives
|
||||
|
||||
8 proven patterns to replace unnecessary Effects:
|
||||
|
||||
1. Calculate during render for derived state
|
||||
2. `useMemo` for expensive calculations
|
||||
3. `key` prop to reset state
|
||||
4. Store ID instead of object for stable references
|
||||
5. Event handlers for user actions
|
||||
6. `useSyncExternalStore` for external stores
|
||||
7. Lifting state up for shared state
|
||||
8. Custom hooks for data fetching with cleanup
|
||||
|
||||
## Usage Examples
|
||||
|
||||
### Example 1: Derived State
|
||||
|
||||
**Bad - Unnecessary Effect:**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const [fullName, setFullName] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName)
|
||||
}, [firstName, lastName])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Calculate during render:**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const fullName = firstName + ' ' + lastName // Just compute it
|
||||
}
|
||||
```
|
||||
|
||||
### Example 2: Resetting State
|
||||
|
||||
**Bad - Effect to reset:**
|
||||
|
||||
```tsx
|
||||
function ProfilePage({ userId }) {
|
||||
const [comment, setComment] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setComment('')
|
||||
}, [userId])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Key prop:**
|
||||
|
||||
```tsx
|
||||
function ProfilePage({ userId }) {
|
||||
return <Profile userId={userId} key={userId} />
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
const [comment, setComment] = useState('') // Resets automatically
|
||||
}
|
||||
```
|
||||
|
||||
### Example 3: Data Fetching with Cleanup
|
||||
|
||||
**Bad - Race condition:**
|
||||
|
||||
```tsx
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
fetchResults(query).then((json) => {
|
||||
setResults(json) // "hello" response may arrive after "hell"
|
||||
})
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Cleanup flag:**
|
||||
|
||||
```tsx
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
|
||||
fetchResults(query).then((json) => {
|
||||
if (!ignore) setResults(json)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
### Example 4: Event Handler Instead of Effect
|
||||
|
||||
**Bad - Effect watching state:**
|
||||
|
||||
```tsx
|
||||
function ProductPage({ product, addToCart }) {
|
||||
useEffect(() => {
|
||||
if (product.isInCart) {
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}, [product])
|
||||
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Handle in event:**
|
||||
|
||||
```tsx
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## When You DO Need Effects
|
||||
|
||||
Effects are appropriate for:
|
||||
|
||||
- **Synchronizing with external systems** - Browser APIs, third-party widgets, non-React code
|
||||
- **Subscriptions** - WebSocket connections, global event listeners (prefer `useSyncExternalStore`)
|
||||
- **Analytics/logging** - Code that needs to run because the component displayed
|
||||
- **Data fetching** - With proper cleanup (or use your framework's built-in mechanism)
|
||||
|
||||
## When You DON'T Need Effects
|
||||
|
||||
Avoid Effects for:
|
||||
|
||||
1. **Transforming data for rendering** - Calculate at the top level instead
|
||||
2. **Handling user events** - Use event handlers where you know exactly what happened
|
||||
3. **Deriving state** - Just compute it: `const fullName = firstName + ' ' + lastName`
|
||||
4. **Chaining state updates** - Calculate all next state in the event handler
|
||||
5. **Notifying parent components** - Call the callback in the same event handler
|
||||
6. **Resetting state** - Use the `key` prop to create a fresh component instance
|
||||
|
||||
## Best Practices
|
||||
|
||||
### 1. Start Without an Effect
|
||||
|
||||
Before adding an Effect, ask: "Is there an external system involved?" If no, you probably don't need an Effect.
|
||||
|
||||
### 2. Prefer Derived State
|
||||
|
||||
If you can calculate a value from props or state, don't store it in state with an Effect updating it.
|
||||
|
||||
### 3. Use the Right Tool
|
||||
|
||||
- Expensive calculation → `useMemo`
|
||||
- User interaction → Event handler
|
||||
- Reset on prop change → `key` prop
|
||||
- External subscription → `useSyncExternalStore`
|
||||
- Shared state → Lift state up
|
||||
|
||||
### 4. Always Clean Up
|
||||
|
||||
If your Effect subscribes, fetches, or sets timers, return a cleanup function to prevent memory leaks and race conditions.
|
||||
|
||||
### 5. Avoid Effect Chains
|
||||
|
||||
Multiple Effects triggering each other causes unnecessary re-renders and makes code hard to follow. Calculate everything in one place (usually an event handler).
|
||||
|
||||
### 6. Test in Strict Mode
|
||||
|
||||
React 18+ Strict Mode mounts components twice in development to expose missing cleanup. If your Effect breaks, you need cleanup.
|
||||
|
||||
### 7. Consider Framework Solutions
|
||||
|
||||
For data fetching, prefer your framework's built-in solution (Next.js, Remix) or libraries (React Query, SWR) over manual Effects.
|
||||
|
||||
## Reference Files
|
||||
|
||||
This skill includes three detailed reference documents:
|
||||
|
||||
1. **SKILL.md** - Quick reference table and decision tree
|
||||
2. **anti-patterns.md** - 9 common mistakes with detailed explanations
|
||||
3. **alternatives.md** - 8 better alternatives with code examples
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Multiple Re-renders
|
||||
|
||||
**Symptom:** Component re-renders many times in quick succession.
|
||||
|
||||
**Cause:** Effect that sets state based on state it depends on, creating a loop.
|
||||
|
||||
**Fix:** Calculate the final value in an event handler or during render.
|
||||
|
||||
### Stale Data
|
||||
|
||||
**Symptom:** UI shows outdated values briefly before updating.
|
||||
|
||||
**Cause:** Using Effect to update derived state causes an extra render pass.
|
||||
|
||||
**Fix:** Calculate derived values during render instead of in state.
|
||||
|
||||
### Race Conditions
|
||||
|
||||
**Symptom:** Fast typing shows results for old queries after new ones.
|
||||
|
||||
**Cause:** Missing cleanup in data fetching Effect.
|
||||
|
||||
**Fix:** Use cleanup flag (`ignore` variable) or AbortController.
|
||||
|
||||
### Runs Twice in Development
|
||||
|
||||
**Symptom:** Effect runs twice on component mount in development.
|
||||
|
||||
**Cause:** React 18 Strict Mode intentionally mounts components twice to expose bugs.
|
||||
|
||||
**Fix:** Add proper cleanup. If it's app initialization that shouldn't run twice, use a module-level guard.
|
||||
|
||||
## Resources
|
||||
|
||||
This skill is based on:
|
||||
|
||||
- [React Official Docs: You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect)
|
||||
- [React Official Docs: Synchronizing with Effects](https://react.dev/learn/synchronizing-with-effects)
|
||||
- [React Official Docs: Lifecycle of Reactive Effects](https://react.dev/learn/lifecycle-of-reactive-effects)
|
||||
|
||||
## Summary
|
||||
|
||||
The golden rule: **Effects are an escape hatch from React.** If you're not synchronizing with an external system, you probably don't need an Effect.
|
||||
|
||||
Before writing `useEffect`, ask yourself:
|
||||
|
||||
1. Is this responding to a user interaction? → Use event handler
|
||||
2. Is this a value I can calculate from props/state? → Calculate during render
|
||||
3. Is this resetting state when a prop changes? → Use key prop
|
||||
4. Is this synchronizing with an external system? → Use Effect with cleanup
|
||||
|
||||
Follow these patterns, and your React code will be more maintainable, performant, and bug-free.
|
||||
53
.agents/skills/react-useeffect/SKILL.md
Normal file
53
.agents/skills/react-useeffect/SKILL.md
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
---
|
||||
name: react-useeffect
|
||||
description: React useEffect best practices from official docs. Use when writing/reviewing useEffect, useState for derived values, data fetching, or state synchronization. Teaches when NOT to use Effect and better alternatives.
|
||||
---
|
||||
|
||||
# You Might Not Need an Effect
|
||||
|
||||
Effects are an **escape hatch** from React. They let you synchronize with external systems. If there is no external system involved, you shouldn't need an Effect.
|
||||
|
||||
## Quick Reference
|
||||
|
||||
| Situation | DON'T | DO |
|
||||
| ------------------------------ | ------------------------------ | ------------------------------------- |
|
||||
| Derived state from props/state | `useState` + `useEffect` | Calculate during render |
|
||||
| Expensive calculations | `useEffect` to cache | `useMemo` |
|
||||
| Reset state on prop change | `useEffect` with `setState` | `key` prop |
|
||||
| User event responses | `useEffect` watching state | Event handler directly |
|
||||
| Notify parent of changes | `useEffect` calling `onChange` | Call in event handler |
|
||||
| Fetch data | `useEffect` without cleanup | `useEffect` with cleanup OR framework |
|
||||
|
||||
## When You DO Need Effects
|
||||
|
||||
- Synchronizing with **external systems** (non-React widgets, browser APIs)
|
||||
- **Subscriptions** to external stores (use `useSyncExternalStore` when possible)
|
||||
- **Analytics/logging** that runs because component displayed
|
||||
- **Data fetching** with proper cleanup (or use framework's built-in mechanism)
|
||||
|
||||
## When You DON'T Need Effects
|
||||
|
||||
1. **Transforming data for rendering** - Calculate at top level, re-runs automatically
|
||||
2. **Handling user events** - Use event handlers, you know exactly what happened
|
||||
3. **Deriving state** - Just compute it: `const fullName = firstName + ' ' + lastName`
|
||||
4. **Chaining state updates** - Calculate all next state in the event handler
|
||||
|
||||
## Decision Tree
|
||||
|
||||
```
|
||||
Need to respond to something?
|
||||
├── User interaction (click, submit, drag)?
|
||||
│ └── Use EVENT HANDLER
|
||||
├── Component appeared on screen?
|
||||
│ └── Use EFFECT (external sync, analytics)
|
||||
├── Props/state changed and need derived value?
|
||||
│ └── CALCULATE DURING RENDER
|
||||
│ └── Expensive? Use useMemo
|
||||
└── Need to reset state when prop changes?
|
||||
└── Use KEY PROP on component
|
||||
```
|
||||
|
||||
## Detailed Guidance
|
||||
|
||||
- [Anti-Patterns](./anti-patterns.md) - Common mistakes with fixes
|
||||
- [Better Alternatives](./alternatives.md) - useMemo, key prop, lifting state, useSyncExternalStore
|
||||
265
.agents/skills/react-useeffect/alternatives.md
Normal file
265
.agents/skills/react-useeffect/alternatives.md
Normal file
|
|
@ -0,0 +1,265 @@
|
|||
# Better Alternatives to useEffect
|
||||
|
||||
## 1. Calculate During Render (Derived State)
|
||||
|
||||
For values derived from props or state, just compute them:
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
|
||||
// Runs every render - that's fine and intentional
|
||||
const fullName = firstName + ' ' + lastName
|
||||
const isValid = firstName.length > 0 && lastName.length > 0
|
||||
}
|
||||
```
|
||||
|
||||
**When to use**: The value can be computed from existing props/state.
|
||||
|
||||
---
|
||||
|
||||
## 2. useMemo for Expensive Calculations
|
||||
|
||||
When computation is expensive, memoize it:
|
||||
|
||||
```tsx
|
||||
import { useMemo } from 'react'
|
||||
|
||||
function TodoList({ todos, filter }) {
|
||||
const visibleTodos = useMemo(() => getFilteredTodos(todos, filter), [todos, filter])
|
||||
}
|
||||
```
|
||||
|
||||
**How to know if it's expensive**:
|
||||
|
||||
```tsx
|
||||
console.time('filter')
|
||||
const visibleTodos = getFilteredTodos(todos, filter)
|
||||
console.timeEnd('filter')
|
||||
// If > 1ms, consider memoizing
|
||||
```
|
||||
|
||||
**Note**: React Compiler can auto-memoize, reducing manual useMemo needs.
|
||||
|
||||
---
|
||||
|
||||
## 3. Key Prop to Reset State
|
||||
|
||||
To reset ALL state when a prop changes, use key:
|
||||
|
||||
```tsx
|
||||
// Parent passes userId as key
|
||||
function ProfilePage({ userId }) {
|
||||
return (
|
||||
<Profile
|
||||
userId={userId}
|
||||
key={userId} // Different userId = different component instance
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
// All state here resets when userId changes
|
||||
const [comment, setComment] = useState('')
|
||||
const [likes, setLikes] = useState([])
|
||||
}
|
||||
```
|
||||
|
||||
**When to use**: You want a "fresh start" when an identity prop changes.
|
||||
|
||||
---
|
||||
|
||||
## 4. Store ID Instead of Object
|
||||
|
||||
To preserve selection when list changes:
|
||||
|
||||
```tsx
|
||||
// BAD: Storing object that needs Effect to "adjust"
|
||||
function List({ items }) {
|
||||
const [selection, setSelection] = useState(null)
|
||||
|
||||
useEffect(() => {
|
||||
setSelection(null) // Reset when items change
|
||||
}, [items])
|
||||
}
|
||||
|
||||
// GOOD: Store ID, derive object
|
||||
function List({ items }) {
|
||||
const [selectedId, setSelectedId] = useState(null)
|
||||
|
||||
// Derived - no Effect needed
|
||||
const selection = items.find((item) => item.id === selectedId) ?? null
|
||||
}
|
||||
```
|
||||
|
||||
**Benefit**: If item with selectedId exists in new list, selection preserved.
|
||||
|
||||
---
|
||||
|
||||
## 5. Event Handlers for User Actions
|
||||
|
||||
User clicks/submits/drags should be handled in event handlers, not Effects:
|
||||
|
||||
```tsx
|
||||
// Event handler knows exactly what happened
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
analytics.track('product_added', { id: product.id })
|
||||
}
|
||||
|
||||
function handleCheckoutClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
navigateTo('/checkout')
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Shared logic**: Extract a function, call from both handlers:
|
||||
|
||||
```tsx
|
||||
function buyProduct() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
|
||||
function handleBuyClick() {
|
||||
buyProduct()
|
||||
}
|
||||
function handleCheckoutClick() {
|
||||
buyProduct()
|
||||
navigateTo('/checkout')
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 6. useSyncExternalStore for External Stores
|
||||
|
||||
For subscribing to external data (browser APIs, third-party stores):
|
||||
|
||||
```tsx
|
||||
// Instead of manual Effect subscription
|
||||
function useOnlineStatus() {
|
||||
const [isOnline, setIsOnline] = useState(true)
|
||||
|
||||
useEffect(() => {
|
||||
function update() {
|
||||
setIsOnline(navigator.onLine)
|
||||
}
|
||||
window.addEventListener('online', update)
|
||||
window.addEventListener('offline', update)
|
||||
return () => {
|
||||
window.removeEventListener('online', update)
|
||||
window.removeEventListener('offline', update)
|
||||
}
|
||||
}, [])
|
||||
|
||||
return isOnline
|
||||
}
|
||||
|
||||
// Use purpose-built hook
|
||||
import { useSyncExternalStore } from 'react'
|
||||
|
||||
function subscribe(callback) {
|
||||
window.addEventListener('online', callback)
|
||||
window.addEventListener('offline', callback)
|
||||
return () => {
|
||||
window.removeEventListener('online', callback)
|
||||
window.removeEventListener('offline', callback)
|
||||
}
|
||||
}
|
||||
|
||||
function useOnlineStatus() {
|
||||
return useSyncExternalStore(
|
||||
subscribe,
|
||||
() => navigator.onLine, // Client value
|
||||
() => true // Server value (SSR)
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Lifting State Up
|
||||
|
||||
When two components need synchronized state, lift it to common ancestor:
|
||||
|
||||
```tsx
|
||||
// Instead of syncing via Effects between siblings
|
||||
function Parent() {
|
||||
const [value, setValue] = useState('')
|
||||
|
||||
return (
|
||||
<>
|
||||
<Input value={value} onChange={setValue} />
|
||||
<Preview value={value} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Custom Hooks for Data Fetching
|
||||
|
||||
Extract fetch logic with proper cleanup:
|
||||
|
||||
```tsx
|
||||
function useData(url) {
|
||||
const [data, setData] = useState(null)
|
||||
const [error, setError] = useState(null)
|
||||
const [loading, setLoading] = useState(true)
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
setLoading(true)
|
||||
|
||||
fetch(url)
|
||||
.then((res) => res.json())
|
||||
.then((json) => {
|
||||
if (!ignore) {
|
||||
setData(json)
|
||||
setError(null)
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
if (!ignore) setError(err)
|
||||
})
|
||||
.finally(() => {
|
||||
if (!ignore) setLoading(false)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [url])
|
||||
|
||||
return { data, error, loading }
|
||||
}
|
||||
|
||||
// Usage
|
||||
function SearchResults({ query }) {
|
||||
const { data, error, loading } = useData(`/api/search?q=${query}`)
|
||||
}
|
||||
```
|
||||
|
||||
**Better**: Use framework's data fetching (React Query, SWR, Next.js, etc.)
|
||||
|
||||
---
|
||||
|
||||
## Summary: When to Use What
|
||||
|
||||
| Need | Solution |
|
||||
| ------------------------------ | ------------------------------------ |
|
||||
| Value from props/state | Calculate during render |
|
||||
| Expensive calculation | `useMemo` |
|
||||
| Reset all state on prop change | `key` prop |
|
||||
| Respond to user action | Event handler |
|
||||
| Sync with external system | `useEffect` with cleanup |
|
||||
| Subscribe to external store | `useSyncExternalStore` |
|
||||
| Share state between components | Lift state up |
|
||||
| Fetch data | Custom hook with cleanup / framework |
|
||||
289
.agents/skills/react-useeffect/anti-patterns.md
Normal file
289
.agents/skills/react-useeffect/anti-patterns.md
Normal file
|
|
@ -0,0 +1,289 @@
|
|||
# useEffect Anti-Patterns
|
||||
|
||||
## 1. Redundant State for Derived Values
|
||||
|
||||
```tsx
|
||||
// BAD: Extra state + Effect for derived value
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const [fullName, setFullName] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName)
|
||||
}, [firstName, lastName])
|
||||
}
|
||||
|
||||
// GOOD: Calculate during rendering
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const fullName = firstName + ' ' + lastName // Just compute it
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Causes extra render pass with stale value, then re-renders with updated value.
|
||||
|
||||
---
|
||||
|
||||
## 2. Filtering/Transforming Data in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to filter list
|
||||
function TodoList({ todos, filter }) {
|
||||
const [visibleTodos, setVisibleTodos] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
setVisibleTodos(getFilteredTodos(todos, filter))
|
||||
}, [todos, filter])
|
||||
}
|
||||
|
||||
// GOOD: Filter during render (memoize if expensive)
|
||||
function TodoList({ todos, filter }) {
|
||||
const visibleTodos = useMemo(() => getFilteredTodos(todos, filter), [todos, filter])
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Resetting State on Prop Change
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to reset state
|
||||
function ProfilePage({ userId }) {
|
||||
const [comment, setComment] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setComment('')
|
||||
}, [userId])
|
||||
}
|
||||
|
||||
// GOOD: Use key prop
|
||||
function ProfilePage({ userId }) {
|
||||
return <Profile userId={userId} key={userId} />
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
const [comment, setComment] = useState('') // Resets automatically
|
||||
}
|
||||
```
|
||||
|
||||
**Why key works**: React treats components with different keys as different components, recreating state.
|
||||
|
||||
---
|
||||
|
||||
## 4. Event-Specific Logic in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect for button click result
|
||||
function ProductPage({ product, addToCart }) {
|
||||
useEffect(() => {
|
||||
if (product.isInCart) {
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}, [product])
|
||||
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD: Handle in event handler
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Effect fires on page refresh (isInCart is true), showing notification unexpectedly.
|
||||
|
||||
---
|
||||
|
||||
## 5. Chains of Effects
|
||||
|
||||
```tsx
|
||||
// BAD: Effects triggering each other
|
||||
function Game() {
|
||||
const [card, setCard] = useState(null)
|
||||
const [goldCardCount, setGoldCardCount] = useState(0)
|
||||
const [round, setRound] = useState(1)
|
||||
const [isGameOver, setIsGameOver] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
if (card?.gold) setGoldCardCount((c) => c + 1)
|
||||
}, [card])
|
||||
|
||||
useEffect(() => {
|
||||
if (goldCardCount > 3) {
|
||||
setRound((r) => r + 1)
|
||||
setGoldCardCount(0)
|
||||
}
|
||||
}, [goldCardCount])
|
||||
|
||||
useEffect(() => {
|
||||
if (round > 5) setIsGameOver(true)
|
||||
}, [round])
|
||||
}
|
||||
|
||||
// GOOD: Calculate in event handler
|
||||
function Game() {
|
||||
const [card, setCard] = useState(null)
|
||||
const [goldCardCount, setGoldCardCount] = useState(0)
|
||||
const [round, setRound] = useState(1)
|
||||
const isGameOver = round > 5 // Derived!
|
||||
|
||||
function handlePlaceCard(nextCard) {
|
||||
if (isGameOver) throw Error('Game ended')
|
||||
|
||||
setCard(nextCard)
|
||||
if (nextCard.gold) {
|
||||
if (goldCardCount < 3) {
|
||||
setGoldCardCount(goldCardCount + 1)
|
||||
} else {
|
||||
setGoldCardCount(0)
|
||||
setRound(round + 1)
|
||||
if (round === 5) alert('Good game!')
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Multiple re-renders (setCard -> setGoldCardCount -> setRound -> setIsGameOver). Also fragile for features like history replay.
|
||||
|
||||
---
|
||||
|
||||
## 6. Notifying Parent via Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to notify parent
|
||||
function Toggle({ onChange }) {
|
||||
const [isOn, setIsOn] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
onChange(isOn)
|
||||
}, [isOn, onChange])
|
||||
|
||||
function handleClick() {
|
||||
setIsOn(!isOn)
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD: Notify in same event
|
||||
function Toggle({ onChange }) {
|
||||
const [isOn, setIsOn] = useState(false)
|
||||
|
||||
function updateToggle(nextIsOn) {
|
||||
setIsOn(nextIsOn)
|
||||
onChange(nextIsOn) // Same event, batched render
|
||||
}
|
||||
|
||||
function handleClick() {
|
||||
updateToggle(!isOn)
|
||||
}
|
||||
}
|
||||
|
||||
// BEST: Fully controlled component
|
||||
function Toggle({ isOn, onChange }) {
|
||||
function handleClick() {
|
||||
onChange(!isOn)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Passing Data Up to Parent
|
||||
|
||||
```tsx
|
||||
// BAD: Child fetches, passes up via Effect
|
||||
function Parent() {
|
||||
const [data, setData] = useState(null)
|
||||
return <Child onFetched={setData} />
|
||||
}
|
||||
|
||||
function Child({ onFetched }) {
|
||||
const data = useSomeAPI()
|
||||
|
||||
useEffect(() => {
|
||||
if (data) onFetched(data)
|
||||
}, [onFetched, data])
|
||||
}
|
||||
|
||||
// GOOD: Parent fetches, passes down
|
||||
function Parent() {
|
||||
const data = useSomeAPI()
|
||||
return <Child data={data} />
|
||||
}
|
||||
```
|
||||
|
||||
**Why**: Data should flow down. Upward flow via Effects makes debugging hard.
|
||||
|
||||
---
|
||||
|
||||
## 8. Fetching Without Cleanup (Race Condition)
|
||||
|
||||
```tsx
|
||||
// BAD: No cleanup - race condition
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
fetchResults(query).then((json) => {
|
||||
setResults(json) // "hello" response may arrive after "hell"
|
||||
})
|
||||
}, [query])
|
||||
}
|
||||
|
||||
// GOOD: Cleanup ignores stale responses
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
|
||||
fetchResults(query).then((json) => {
|
||||
if (!ignore) setResults(json)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 9. App Initialization in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Runs twice in dev, may break auth
|
||||
function App() {
|
||||
useEffect(() => {
|
||||
loadDataFromLocalStorage()
|
||||
checkAuthToken() // May invalidate token on second call!
|
||||
}, [])
|
||||
}
|
||||
|
||||
// GOOD: Module-level guard
|
||||
let didInit = false
|
||||
|
||||
function App() {
|
||||
useEffect(() => {
|
||||
if (!didInit) {
|
||||
didInit = true
|
||||
loadDataFromLocalStorage()
|
||||
checkAuthToken()
|
||||
}
|
||||
}, [])
|
||||
}
|
||||
|
||||
// ALSO GOOD: Module-level execution
|
||||
if (typeof window !== 'undefined') {
|
||||
checkAuthToken()
|
||||
loadDataFromLocalStorage()
|
||||
}
|
||||
```
|
||||
1
.claude/skills/react-useeffect
Symbolic link
1
.claude/skills/react-useeffect
Symbolic link
|
|
@ -0,0 +1 @@
|
|||
../../.agents/skills/react-useeffect
|
||||
|
|
@ -5,6 +5,11 @@
|
|||
"source": "vercel-labs/agent-browser",
|
||||
"sourceType": "github",
|
||||
"computedHash": "587f7d9187c237d909b9f446cf3ac357184342d54ceb7c430dec56fdbac6816c"
|
||||
},
|
||||
"react-useeffect": {
|
||||
"source": "softaworks/agent-toolkit",
|
||||
"sourceType": "github",
|
||||
"computedHash": "8c24990b50f431a570f3ad1f60632dee154431861547103047f6ce80092ca734"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -55,7 +55,6 @@ function UIZoomControl(): React.JSX.Element {
|
|||
useEffect(() => {
|
||||
return window.api.ui.onTerminalZoom(() => {
|
||||
setZoomLevel(window.api.ui.getZoomLevel())
|
||||
setTimeout(() => setZoomLevel(window.api.ui.getZoomLevel()), 50)
|
||||
})
|
||||
}, [])
|
||||
|
||||
|
|
@ -306,12 +305,14 @@ function FontAutocomplete({
|
|||
onChange
|
||||
}: FontAutocompleteProps): React.JSX.Element {
|
||||
const [query, setQuery] = useState(value)
|
||||
const [prevValue, setPrevValue] = useState(value)
|
||||
const [open, setOpen] = useState(false)
|
||||
const rootRef = useRef<HTMLDivElement | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
if (value !== prevValue) {
|
||||
setPrevValue(value)
|
||||
setQuery(value)
|
||||
}, [value])
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) return
|
||||
|
|
@ -442,6 +443,7 @@ function Settings(): React.JSX.Element {
|
|||
const [themeSearchLight, setThemeSearchLight] = useState('')
|
||||
const [systemPrefersDark, setSystemPrefersDark] = useState(getSystemPrefersDark())
|
||||
const [scrollbackMode, setScrollbackMode] = useState<'preset' | 'custom'>('preset')
|
||||
const [prevSettings, setPrevSettings] = useState(settings)
|
||||
const [terminalFontSuggestions, setTerminalFontSuggestions] = useState<string[]>(
|
||||
getFallbackTerminalFonts()
|
||||
)
|
||||
|
|
@ -490,16 +492,17 @@ function Settings(): React.JSX.Element {
|
|||
}
|
||||
}, [selectedPane])
|
||||
|
||||
useEffect(() => {
|
||||
if (!settings) return
|
||||
|
||||
const scrollbackMb = Math.max(1, Math.round(settings.terminalScrollbackBytes / 1_000_000))
|
||||
setScrollbackMode(
|
||||
SCROLLBACK_PRESETS_MB.includes(scrollbackMb as (typeof SCROLLBACK_PRESETS_MB)[number])
|
||||
? 'preset'
|
||||
: 'custom'
|
||||
)
|
||||
}, [settings])
|
||||
if (settings !== prevSettings) {
|
||||
setPrevSettings(settings)
|
||||
if (settings) {
|
||||
const scrollbackMb = Math.max(1, Math.round(settings.terminalScrollbackBytes / 1_000_000))
|
||||
setScrollbackMode(
|
||||
SCROLLBACK_PRESETS_MB.includes(scrollbackMb as (typeof SCROLLBACK_PRESETS_MB)[number])
|
||||
? 'preset'
|
||||
: 'custom'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
let stale = false
|
||||
|
|
@ -603,17 +606,15 @@ function Settings(): React.JSX.Element {
|
|||
}
|
||||
}, [selectedRepoId, baseRefQuery])
|
||||
|
||||
useEffect(() => {
|
||||
if (repos.length === 0) {
|
||||
// Validate selectedRepoId against current repos (adjusting state during render)
|
||||
if (repos.length === 0) {
|
||||
if (selectedRepoId !== null) {
|
||||
setSelectedRepoId(null)
|
||||
if (selectedPane === 'repo') setSelectedPane('general')
|
||||
return
|
||||
}
|
||||
|
||||
if (!selectedRepoId || !repos.some((repo) => repo.id === selectedRepoId)) {
|
||||
setSelectedRepoId(repos[0].id)
|
||||
}
|
||||
}, [repos, selectedRepoId])
|
||||
} else if (!selectedRepoId || !repos.some((repo) => repo.id === selectedRepoId)) {
|
||||
setSelectedRepoId(repos[0].id)
|
||||
}
|
||||
|
||||
const applyTheme = useCallback((theme: 'system' | 'dark' | 'light') => {
|
||||
const root = document.documentElement
|
||||
|
|
|
|||
|
|
@ -25,6 +25,11 @@ export default function Terminal(): React.JSX.Element | null {
|
|||
const allWorktrees = Object.values(worktreesByRepo).flat()
|
||||
const prevTabCountRef = useRef(tabs.length)
|
||||
|
||||
// Ensure activeTabId is valid (adjusting state during render)
|
||||
if (tabs.length > 0 && (!activeTabId || !tabs.find((t) => t.id === activeTabId))) {
|
||||
setActiveTab(tabs[0].id)
|
||||
}
|
||||
|
||||
// Track which worktrees have been activated during this app session.
|
||||
// Only mount TerminalPanes for visited worktrees to prevent mass PTY
|
||||
// spawning when restoring a session with many saved worktree tabs.
|
||||
|
|
@ -57,13 +62,6 @@ export default function Terminal(): React.JSX.Element | null {
|
|||
createTab(activeWorktreeId)
|
||||
}, [workspaceSessionReady, activeWorktreeId, tabs.length, createTab])
|
||||
|
||||
// Ensure activeTabId is valid
|
||||
useEffect(() => {
|
||||
if (tabs.length > 0 && (!activeTabId || !tabs.find((t) => t.id === activeTabId))) {
|
||||
setActiveTab(tabs[0].id)
|
||||
}
|
||||
}, [tabs, activeTabId, setActiveTab])
|
||||
|
||||
// Animate tab bar height with grid transition
|
||||
useEffect(() => {
|
||||
const el = tabBarRef.current
|
||||
|
|
|
|||
|
|
@ -49,9 +49,9 @@ export default function TerminalSearch({
|
|||
return
|
||||
}
|
||||
if (searchAddon && isOpen) {
|
||||
searchAddon.findNext(query, searchOptions(true))
|
||||
searchAddon.findNext(query, { caseSensitive, regex, incremental: true })
|
||||
}
|
||||
}, [query, searchAddon, isOpen, searchOptions])
|
||||
}, [query, searchAddon, isOpen, caseSensitive, regex])
|
||||
|
||||
const handleKeyDown = useCallback(
|
||||
(e: React.KeyboardEvent) => {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,8 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
|
|||
const nameInputRef = useRef<HTMLInputElement>(null)
|
||||
const lastSuggestedNameRef = useRef('')
|
||||
const resetTimeoutRef = useRef<number | null>(null)
|
||||
const prevIsOpenRef = useRef(false)
|
||||
const prevSuggestedNameRef = useRef('')
|
||||
|
||||
const isOpen = activeModal === 'create-worktree'
|
||||
const preselectedRepoId =
|
||||
|
|
@ -65,6 +67,33 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
|
|||
[repoId, worktreesByRepo, settings?.nestWorkspaces]
|
||||
)
|
||||
|
||||
// Auto-select repo when dialog opens (adjusting state during render)
|
||||
if (isOpen && !prevIsOpenRef.current && repos.length > 0) {
|
||||
if (preselectedRepoId && repos.some((repo) => repo.id === preselectedRepoId)) {
|
||||
setRepoId(preselectedRepoId)
|
||||
} else if (activeWorktreeRepoId && repos.some((repo) => repo.id === activeWorktreeRepoId)) {
|
||||
setRepoId(activeWorktreeRepoId)
|
||||
} else if (activeRepoId && repos.some((repo) => repo.id === activeRepoId)) {
|
||||
setRepoId(activeRepoId)
|
||||
} else {
|
||||
setRepoId(repos[0].id)
|
||||
}
|
||||
}
|
||||
prevIsOpenRef.current = isOpen
|
||||
|
||||
// Auto-fill name from suggestion (adjusting state during render)
|
||||
if (isOpen && repoId && suggestedName && suggestedName !== prevSuggestedNameRef.current) {
|
||||
const shouldApplySuggestion = !name.trim() || name === lastSuggestedNameRef.current
|
||||
prevSuggestedNameRef.current = suggestedName
|
||||
if (shouldApplySuggestion) {
|
||||
setName(suggestedName)
|
||||
lastSuggestedNameRef.current = suggestedName
|
||||
}
|
||||
}
|
||||
if (!isOpen) {
|
||||
prevSuggestedNameRef.current = ''
|
||||
}
|
||||
|
||||
const handleOpenChange = useCallback(
|
||||
(open: boolean) => {
|
||||
if (!open) {
|
||||
|
|
@ -151,45 +180,16 @@ const AddWorktreeDialog = React.memo(function AddWorktreeDialog() {
|
|||
}
|
||||
}, [isOpen])
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!isOpen || repos.length === 0) return
|
||||
|
||||
if (preselectedRepoId && repos.some((repo) => repo.id === preselectedRepoId)) {
|
||||
setRepoId(preselectedRepoId)
|
||||
return
|
||||
}
|
||||
|
||||
if (activeWorktreeRepoId && repos.some((repo) => repo.id === activeWorktreeRepoId)) {
|
||||
setRepoId(activeWorktreeRepoId)
|
||||
return
|
||||
}
|
||||
|
||||
if (activeRepoId && repos.some((repo) => repo.id === activeRepoId)) {
|
||||
setRepoId(activeRepoId)
|
||||
return
|
||||
}
|
||||
|
||||
if (!repoId) {
|
||||
setRepoId(repos[0].id)
|
||||
}
|
||||
}, [isOpen, repos, repoId, preselectedRepoId, activeWorktreeRepoId, activeRepoId])
|
||||
|
||||
// Focus and select name input when suggestion is applied
|
||||
React.useEffect(() => {
|
||||
if (!isOpen || !repoId || !suggestedName) return
|
||||
|
||||
const shouldApplySuggestion = !name.trim() || name === lastSuggestedNameRef.current
|
||||
if (!shouldApplySuggestion) return
|
||||
|
||||
setName(suggestedName)
|
||||
lastSuggestedNameRef.current = suggestedName
|
||||
|
||||
requestAnimationFrame(() => {
|
||||
const input = nameInputRef.current
|
||||
if (!input) return
|
||||
input.focus()
|
||||
input.select()
|
||||
})
|
||||
}, [isOpen, repoId, suggestedName, name])
|
||||
}, [isOpen, repoId, suggestedName])
|
||||
|
||||
// Safety guard: creating a worktree requires at least one repo.
|
||||
React.useEffect(() => {
|
||||
|
|
|
|||
|
|
@ -269,23 +269,6 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
return result
|
||||
}, [groupBy, worktrees, repoMap, prCache, collapsedGroups, tabsByWorktree])
|
||||
|
||||
React.useEffect(() => {
|
||||
if (!pendingRevealWorktreeId || groupBy === 'none') return
|
||||
|
||||
const targetWorktree = worktrees.find((worktree) => worktree.id === pendingRevealWorktreeId)
|
||||
if (!targetWorktree) return
|
||||
|
||||
const groupKey = getGroupKeyForWorktree(groupBy, targetWorktree, repoMap, prCache)
|
||||
if (!groupKey) return
|
||||
|
||||
setCollapsedGroups((prev) => {
|
||||
if (!prev.has(groupKey)) return prev
|
||||
const next = new Set(prev)
|
||||
next.delete(groupKey)
|
||||
return next
|
||||
})
|
||||
}, [pendingRevealWorktreeId, groupBy, worktrees, repoMap, prCache])
|
||||
|
||||
// ── TanStack Virtual ──────────────────────────────────────────
|
||||
const virtualizer = useVirtualizer({
|
||||
count: rows.length,
|
||||
|
|
@ -301,14 +284,42 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
React.useEffect(() => {
|
||||
if (!pendingRevealWorktreeId) return
|
||||
|
||||
const targetIndex = rows.findIndex(
|
||||
(row) => row.type === 'item' && row.worktree.id === pendingRevealWorktreeId
|
||||
)
|
||||
if (targetIndex === -1) return
|
||||
// Uncollapse the group containing the target worktree
|
||||
if (groupBy !== 'none') {
|
||||
const targetWorktree = worktrees.find((w) => w.id === pendingRevealWorktreeId)
|
||||
if (targetWorktree) {
|
||||
const groupKey = getGroupKeyForWorktree(groupBy, targetWorktree, repoMap, prCache)
|
||||
if (groupKey) {
|
||||
setCollapsedGroups((prev) => {
|
||||
if (!prev.has(groupKey)) return prev
|
||||
const next = new Set(prev)
|
||||
next.delete(groupKey)
|
||||
return next
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
virtualizer.scrollToIndex(targetIndex, { align: 'center' })
|
||||
clearPendingRevealWorktreeId()
|
||||
}, [pendingRevealWorktreeId, rows, virtualizer, clearPendingRevealWorktreeId])
|
||||
// Scroll to the target after the group uncollapse re-render settles
|
||||
requestAnimationFrame(() => {
|
||||
const targetIndex = rows.findIndex(
|
||||
(row) => row.type === 'item' && row.worktree.id === pendingRevealWorktreeId
|
||||
)
|
||||
if (targetIndex !== -1) {
|
||||
virtualizer.scrollToIndex(targetIndex, { align: 'center' })
|
||||
}
|
||||
clearPendingRevealWorktreeId()
|
||||
})
|
||||
}, [
|
||||
pendingRevealWorktreeId,
|
||||
groupBy,
|
||||
worktrees,
|
||||
repoMap,
|
||||
prCache,
|
||||
rows,
|
||||
virtualizer,
|
||||
clearPendingRevealWorktreeId
|
||||
])
|
||||
|
||||
const handleCreateForRepo = useCallback(
|
||||
(repoId: string) => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useCallback, useEffect, useMemo, useState } from 'react'
|
||||
import React, { useCallback, useMemo, useRef, useState } from 'react'
|
||||
import { useAppStore } from '@/store'
|
||||
import {
|
||||
Dialog,
|
||||
|
|
@ -32,11 +32,12 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() {
|
|||
const [commentInput, setCommentInput] = useState('')
|
||||
const [saving, setSaving] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
if (!isOpen) return
|
||||
const prevIsOpenRef = useRef(false)
|
||||
if (isOpen && !prevIsOpenRef.current) {
|
||||
setIssueInput(currentIssue)
|
||||
setCommentInput(currentComment)
|
||||
}, [isOpen, currentIssue, currentComment])
|
||||
}
|
||||
prevIsOpenRef.current = isOpen
|
||||
|
||||
const canSave = useMemo(() => {
|
||||
if (!worktreeId) return false
|
||||
|
|
|
|||
Loading…
Reference in a new issue