Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 25 additions & 8 deletions .claude/commands/you-might-not-need-a-callback.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,34 @@ User arguments: $ARGUMENTS
Read before analyzing:
1. https://react.dev/reference/react/useCallback — official docs on when useCallback is actually needed

## The one rule that matters

`useCallback` is only useful when **something observes the reference**. Ask: does anything care if this function gets a new identity on re-render?

Observers that care about reference stability:
- A `useEffect` that lists the function in its deps array
- A `useMemo` that lists the function in its deps array
- Another `useCallback` that lists the function in its deps array
- A child component wrapped in `React.memo` that receives the function as a prop

If none of those apply — if the function is only called inline, or passed to a non-memoized child, or assigned to a native element event — the reference is unobserved and `useCallback` adds overhead with zero benefit.

## Anti-patterns to detect

1. **useCallback on functions not passed as props or deps**: No benefit if only called within the same component.
2. **useCallback with deps that change every render**: Memoization is wasted.
3. **useCallback on handlers passed to native elements**: `<button onClick={fn}>` doesn't benefit from stable references.
4. **useCallback wrapping functions that return new objects/arrays**: Memoization at the wrong level.
5. **useCallback with empty deps when deps are needed**: Stale closures.
6. **Pairing useCallback + React.memo unnecessarily**: Only optimize when you've measured a problem.
7. **useCallback in hooks that don't need stable references**: Not every hook return needs memoization.
1. **No observer tracks the reference**: The function is only called inline in the same component, or passed to a non-memoized child, or used as a native element handler (`<button onClick={fn}>`). Nothing re-runs or bails out based on reference identity. Remove `useCallback`.
2. **useCallback with deps that change every render**: If a dep is a plain object/array created inline, or state that changes on every interaction, memoization buys nothing — the function gets a new identity anyway.
3. **useCallback on handlers passed only to native elements**: `<button onClick={fn}>` — React never does reference equality on native element props. No benefit.
4. **useCallback wrapping functions that return new objects/arrays**: Stable function identity, unstable return value — memoization is at the wrong level. Use `useMemo` on the return value instead, or restructure.
5. **useCallback with empty deps when deps are needed**: Stale closure — reads initial values forever. This is a correctness bug, not just a performance issue.
6. **Pairing useCallback + React.memo on trivially cheap renders**: If the child renders in < 1ms and re-renders rarely, the memo infrastructure costs more than it saves.

## Patterns that ARE correct — do not flag

Note: This codebase uses a ref pattern for stable callbacks (`useRef` + empty deps). That pattern is correct — don't flag it.
- `useCallback` whose result is in a `useEffect` dep array — prevents the effect from re-running on every render
- `useCallback` whose result is in a `useMemo` dep array — prevents the memo from recomputing on every render
- `useCallback` whose result is a dep of another `useCallback` — stabilises a callback chain
- `useCallback` passed to a `React.memo`-wrapped child — the whole point of the pattern
- This codebase's ref pattern: `useRef` + callback with empty deps that reads the ref inside — correct, do not flag

## Steps

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ export function InlineRenameInput({ value, onChange, onSubmit, onCancel }: Inlin
ref={inputRef}
type='text'
value={value}
size={Math.max(value.length + 2, 5)}
onChange={(e) => onChange(e.target.value)}
onKeyDown={(e) => {
if (e.key === 'Enter') onSubmit()
if (e.key === 'Escape') onCancel()
}}
onBlur={onSubmit}
onClick={(e) => e.stopPropagation()}
className='min-w-0 flex-1 truncate border-0 bg-transparent p-0 font-medium text-[var(--text-body)] text-sm outline-none focus:outline-none focus:ring-0'
className='min-w-0 border-0 bg-transparent p-0 font-medium text-[var(--text-body)] text-sm outline-none focus:outline-none focus:ring-0'
/>
)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use client'

import { useCallback, useEffect, useRef, useState } from 'react'
import { memo, useEffect, useRef, useState } from 'react'
import {
Button,
Check,
Expand Down Expand Up @@ -50,7 +50,12 @@ interface MessageActionsProps {
requestId?: string
}

export function MessageActions({ content, chatId, userQuery, requestId }: MessageActionsProps) {
export const MessageActions = memo(function MessageActions({
content,
chatId,
userQuery,
requestId,
}: MessageActionsProps) {
const [copied, setCopied] = useState(false)
const [copiedRequestId, setCopiedRequestId] = useState(false)
const [pendingFeedback, setPendingFeedback] = useState<'up' | 'down' | null>(null)
Expand All @@ -70,7 +75,7 @@ export function MessageActions({ content, chatId, userQuery, requestId }: Messag
}
}, [])

const copyToClipboard = useCallback(async () => {
const copyToClipboard = async () => {
if (!content) return
const text = toPlainText(content)
if (!text) return
Expand All @@ -84,9 +89,9 @@ export function MessageActions({ content, chatId, userQuery, requestId }: Messag
} catch {
/* clipboard unavailable */
}
}, [content])
}

const copyRequestId = useCallback(async () => {
const copyRequestId = async () => {
if (!requestId) return
try {
await navigator.clipboard.writeText(requestId)
Expand All @@ -98,20 +103,17 @@ export function MessageActions({ content, chatId, userQuery, requestId }: Messag
} catch {
/* clipboard unavailable */
}
}, [requestId])
}

const handleFeedbackClick = useCallback(
(type: 'up' | 'down') => {
if (chatId && userQuery) {
setPendingFeedback(type)
setFeedbackText('')
setCopiedRequestId(false)
}
},
[chatId, userQuery]
)
const handleFeedbackClick = (type: 'up' | 'down') => {
if (chatId && userQuery) {
setPendingFeedback(type)
setFeedbackText('')
setCopiedRequestId(false)
}
}

const handleSubmitFeedback = useCallback(() => {
const handleSubmitFeedback = () => {
if (!pendingFeedback || !chatId || !userQuery) return
const text = feedbackText.trim()
if (!text) {
Expand All @@ -128,15 +130,15 @@ export function MessageActions({ content, chatId, userQuery, requestId }: Messag
})
setPendingFeedback(null)
setFeedbackText('')
}, [pendingFeedback, chatId, userQuery, content, feedbackText])
}

const handleModalClose = useCallback((open: boolean) => {
const handleModalClose = (open: boolean) => {
if (!open) {
setPendingFeedback(null)
setFeedbackText('')
setCopiedRequestId(false)
}
}, [])
}

if (!content) return null

Expand Down Expand Up @@ -224,4 +226,4 @@ export function MessageActions({ content, chatId, userQuery, requestId }: Messag
</Modal>
</>
)
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export interface HeaderAction {
icon?: React.ElementType
onClick: () => void
disabled?: boolean
active?: boolean
}

export interface CreateAction {
Expand Down Expand Up @@ -103,7 +104,13 @@ export const ResourceHeader = memo(function ResourceHeader({
onClick={action.onClick}
disabled={action.disabled}
variant='subtle'
className='px-2 py-1 text-caption'
className={cn(
'px-2 py-1 text-caption',
action.active !== undefined && 'rounded-lg',
action.active === true &&
'bg-[var(--surface-active)] hover-hover:bg-[var(--surface-active)]',
action.active === false && 'hover-hover:bg-[var(--surface-hover)]'
)}
>
{ActionIcon && (
<ActionIcon
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ const SortDropdown = memo(function SortDropdown({ config }: { config: SortConfig
Sort
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align='end'>
<DropdownMenuContent align='start'>
{options.map((option) => {
const isActive = active?.column === option.id
const Icon = option.icon
Expand Down
Loading
Loading