Skip to content

Commit 24e8783

Browse files
committed
refactor(FR-2616): rely on React Compiler ('use memo') for memoization
Drop manual useMemo / useCallback usage across the Story 1 code and add the 'use memo' directive to every component and hook body. Under React 19.2 with babel-plugin-react-compiler in annotation mode, the compiler handles identity caching automatically; manual dependency arrays add noise and drift risk without improving the output. Also add .claude/rules/react-compiler-memoization.md documenting the rule so future contributors (human and agent) adopt the same pattern consistently. The rule references the existing use-effect-event.md and points to CLAUDE.md for the project-wide convention. Files touched: - react/src/hooks/useSToken.ts: unwrap useCallback-wrapped clear; drop useCallback import. - react/src/components/STokenLoginBoundary.tsx: add 'use memo' to STokenLoginBoundary outer, DefaultFallback, DefaultErrorCard; unwrap useCallback on retry/handleRetry/handleCopy; drop useCallback import. - .claude/rules/react-compiler-memoization.md (new). Existing tests and the URL-free CI grep continue to pass. Refs FR-2616
1 parent 61ad3d1 commit 24e8783

3 files changed

Lines changed: 133 additions & 22 deletions

File tree

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
---
2+
description: When writing or editing React components and hooks that need memoization
3+
---
4+
5+
# React Compiler Memoization Rule
6+
7+
This project runs **React 19.2** with **`babel-plugin-react-compiler` in annotation mode**. Memoization is the React Compiler's job, not the developer's. Do not reach for `useMemo` / `useCallback` as a first instinct — add the `'use memo'` directive to the component or hook body instead, and write plain values and plain functions.
8+
9+
## Why
10+
11+
1. The compiler analyzes dependencies and inserts caching automatically, including cases that are hard or impossible to express with manual `useMemo` / `useCallback` dependency arrays (e.g., stable references across conditional code paths, partial memoization of branches).
12+
2. Manual dependency arrays drift out of sync as code evolves — the compiler's view is always up to date with the code it compiles.
13+
3. `useCallback` cascades: wrapping one function in `useCallback` forces every caller that depends on its identity (child components, other hooks) to also memoize. The compiler breaks this cascade.
14+
4. Inline arrow functions passed to JSX (`onClick`, `action`, etc.) are already memoized by the compiler under `'use memo'`; wrapping them is noise.
15+
5. Consistent adoption across the codebase keeps the memoization story legible — reviewers don't have to decide case-by-case whether a given `useCallback` is load-bearing or ceremonial.
16+
17+
## Rules
18+
19+
1. **Add `'use memo'` at the top of every component body and every custom hook body** that would benefit from memoization. This is almost all of them. Never remove an existing `'use memo'` directive.
20+
2. **Do not introduce `useMemo` or `useCallback`** in new code. Replace existing usages with plain values and plain functions when you touch them, and add `'use memo'` if the enclosing function doesn't already have it.
21+
3. **Callbacks passed to JSX** (`onClick`, `onChange`, `action`, `onSubmit`, …) must be plain inline or named functions. The compiler memoizes them.
22+
4. **Values derived from props / state** should be plain `const` declarations. If the derivation is expensive *and* measurably slow, consider extracting to a separate module or a hook — but still no `useMemo`.
23+
5. **Exception — `useEffectEvent`**: when you need a callback inside an effect that should read the latest props/state without triggering re-runs, use `useEffectEvent` (see `use-effect-event.md`). This is complementary to `'use memo'` and does not replace it.
24+
6. **Exception — identity-sensitive external APIs**: if a third-party API requires a stable reference across renders (e.g., a subscription that can only be installed once), prefer `useRef` or an effect-scoped closure over `useCallback`. `useCallback`'s identity guarantees are still weaker than the compiler's.
25+
26+
## Pattern
27+
28+
### ✅ Correct — plain functions, `'use memo'` directive
29+
30+
```tsx
31+
const UserProfile: React.FC<Props> = ({ user, onSelect }) => {
32+
'use memo';
33+
const { t } = useTranslation();
34+
35+
const fullName = `${user.firstName} ${user.lastName}`;
36+
37+
const handleClick = () => {
38+
onSelect(user.id);
39+
};
40+
41+
return (
42+
<button onClick={handleClick}>
43+
{t('userProfile.Greet', { name: fullName })}
44+
</button>
45+
);
46+
};
47+
```
48+
49+
### ✅ Correct — custom hook with plain returned functions
50+
51+
```tsx
52+
export const useSomething = (): [Value, (next: Value) => void] => {
53+
'use memo';
54+
const [value, setValue] = useState<Value>(initial);
55+
56+
const update = (next: Value) => {
57+
setValue(next);
58+
// side effects here, reading latest closure values
59+
};
60+
61+
return [value, update];
62+
};
63+
```
64+
65+
### ❌ Wrong — manual `useCallback` wrapping a simple handler
66+
67+
```tsx
68+
const UserProfile: React.FC<Props> = ({ user, onSelect }) => {
69+
// no 'use memo'
70+
const handleClick = useCallback(() => {
71+
onSelect(user.id);
72+
}, [onSelect, user.id]);
73+
74+
return <button onClick={handleClick}>...</button>;
75+
};
76+
```
77+
78+
### ❌ Wrong — `useMemo` for trivial derivations
79+
80+
```tsx
81+
const UserProfile: React.FC<Props> = ({ user }) => {
82+
// no 'use memo'
83+
const fullName = useMemo(
84+
() => `${user.firstName} ${user.lastName}`,
85+
[user.firstName, user.lastName],
86+
);
87+
return <span>{fullName}</span>;
88+
};
89+
```
90+
91+
## Migration
92+
93+
When editing a file that contains `useMemo` or `useCallback`:
94+
95+
1. Add `'use memo'` to the enclosing component / hook body if it is not already there.
96+
2. Unwrap each `useMemo(() => expr, deps)` into `const value = expr;`.
97+
3. Unwrap each `useCallback(fn, deps)` into the original function body, as a plain `const fn = (...)` or inline in JSX.
98+
4. Drop the `useMemo` / `useCallback` import if it becomes unused.
99+
5. Run `bash scripts/verify.sh` — the React Compiler is part of the build pipeline; if the compiler cannot safely memoize something (e.g., mutation in a closure), the lint rule `react-hooks/*` or the build itself will surface the problem.
100+
101+
## Verification
102+
103+
After editing a memoization-relevant file, confirm:
104+
105+
- `'use memo'` appears on the first line of the component / hook body.
106+
- No new `useMemo` or `useCallback` imports or calls were introduced.
107+
- `bash scripts/verify.sh` passes — the compiler is exercised during `pnpm run lint` and the TypeScript build.
108+
- For hooks: existing call sites still behave correctly. The compiler's output is observably equivalent to the hand-rolled version in almost every case; if not, the compiler surfaces a diagnostic.
109+
110+
## Related
111+
112+
- `use-effect-event.md` — companion rule for the `useEffectEvent` primitive used inside effects.
113+
- `CLAUDE.md` → "React Essentials" — project-wide React conventions, including the `'use memo'` requirement.

react/src/components/STokenLoginBoundary.tsx

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { BAIButton, BAICard, BAIFlex, useBAILogger } from 'backend.ai-ui';
2424
import { useAtomValue } from 'jotai';
2525
import {
2626
Suspense,
27-
useCallback,
2827
useEffect,
2928
useEffectEvent,
3029
useRef,
@@ -103,6 +102,7 @@ type Phase =
103102
export const STokenLoginBoundary: React.FC<STokenLoginBoundaryProps> = (
104103
props,
105104
) => {
105+
'use memo';
106106
return (
107107
<Suspense fallback={props.fallback ?? <DefaultFallback />}>
108108
<STokenLoginBoundaryInner {...props} />
@@ -236,10 +236,10 @@ const STokenLoginBoundaryInner: React.FC<STokenLoginBoundaryProps> = ({
236236
runLoginSequence();
237237
}, [retryKey]);
238238

239-
const retry = useCallback(() => {
239+
const retry = () => {
240240
setPhase({ name: 'pending' });
241241
setRetryKey((k) => k + 1);
242-
}, []);
242+
};
243243

244244
if (phase.name === 'error') {
245245
if (errorFallback) {
@@ -274,6 +274,7 @@ const kindToI18nKey = (kind: STokenLoginError['kind']): string =>
274274
* working but hasn't failed.
275275
*/
276276
const DefaultFallback: React.FC = () => {
277+
'use memo';
277278
const { t } = useTranslation();
278279
return (
279280
<BAIFlex
@@ -308,6 +309,7 @@ const DefaultErrorCard: React.FC<{
308309
error: STokenLoginError;
309310
onRetry: () => void;
310311
}> = ({ error, onRetry }) => {
312+
'use memo';
311313
const { t } = useTranslation();
312314
const { message } = App.useApp();
313315

@@ -319,23 +321,22 @@ const DefaultErrorCard: React.FC<{
319321
? String((error.cause as Error)?.message ?? error.cause)
320322
: null;
321323

322-
const handleRetry = useCallback(async () => {
323-
// Wrap in a Promise so BAIButton.action triggers its async loading
324-
// state; the synchronous state reset completes before the next
325-
// render, which is visually indistinguishable from the live
326-
// sequence restart.
324+
// Wrap in a Promise so BAIButton.action triggers its async loading
325+
// state; the synchronous state reset completes before the next render,
326+
// which is visually indistinguishable from the live sequence restart.
327+
const handleRetry = async () => {
327328
await Promise.resolve();
328329
onRetry();
329-
}, [onRetry]);
330+
};
330331

331-
const handleCopy = useCallback(async () => {
332+
const handleCopy = async () => {
332333
const payload = {
333334
kind: error.kind,
334335
cause: causeDetail,
335336
};
336337
await navigator.clipboard.writeText(JSON.stringify(payload, null, 2));
337338
message.success(t('sTokenLoginBoundary.ErrorDetailsCopied'));
338-
}, [error, causeDetail, message, t]);
339+
};
339340

340341
return (
341342
<BAIFlex

react/src/hooks/useSToken.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55
import { useBAILogger } from 'backend.ai-ui';
66
import { parseAsString, useQueryState } from 'nuqs';
7-
import { useCallback, useEffect, useEffectEvent, useRef } from 'react';
7+
import { useEffect, useEffectEvent, useRef } from 'react';
88

99
/**
1010
* Read and clear the sToken URL query parameter using nuqs.
@@ -50,16 +50,13 @@ export const useSToken = (): [
5050
logDeprecationIfNeeded();
5151
}, [sToken, stoken]);
5252

53-
const clear = useCallback(
54-
async (next: string | null) => {
55-
// Clear both keys; callers typically pass `null` after a successful
56-
// login to strip the token from the URL. Return the final search
57-
// params from the last setter to match nuqs' Promise contract.
58-
await setSToken(next);
59-
return setStoken(null);
60-
},
61-
[setSToken, setStoken],
62-
);
53+
// React Compiler ('use memo') memoizes the returned tuple; no manual
54+
// useCallback wrapper is required. Callers typically pass `null` after a
55+
// successful login to strip the token from the URL.
56+
const clear = async (next: string | null) => {
57+
await setSToken(next);
58+
return setStoken(null);
59+
};
6360

6461
const effective = sToken ?? stoken;
6562
return [effective, clear];

0 commit comments

Comments
 (0)