Skip to content

Commit e109675

Browse files
committed
refactor(FR-2616): rely on React Compiler ('use memo') for memoization (#6857)
resolves #NNN (FR-MMM) <!-- replace NNN, MMM with the GitHub issue number and the corresponding Jira issue number. --> <!-- Please precisely, concisely, and concretely describe what this PR changes, the rationale behind codes, and how it affects the users and other developers. --> **Checklist:** (if applicable) - [ ] Documentation - [ ] Minium required manager version - [ ] Specific setting for review (eg., KB link, endpoint or how to setup) - [ ] Minimum requirements to check during review - [ ] Test case(s) to demonstrate the difference of before/after
1 parent f96e136 commit e109675

4 files changed

Lines changed: 138 additions & 25 deletions

File tree

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
---
2+
description: Rely on the React Compiler (`'use memo'`) instead of manual `useMemo` / `useCallback`
3+
paths:
4+
- "react/**/*.{tsx,ts}"
5+
- "packages/backend.ai-ui/**/*.{tsx,ts}"
6+
---
7+
8+
# React Compiler Memoization Rule
9+
10+
This project runs **React 19.2** with **`babel-plugin-react-compiler` in annotation mode**. The compiler owns memoization. Add the `'use memo'` directive to each component and hook body, and write plain values and plain functions.
11+
12+
## Why
13+
14+
- Manual `useMemo` / `useCallback` dependency arrays drift out of sync as code evolves; the compiler's view is always current with the code it compiles.
15+
- `useCallback` forces every identity-sensitive caller (child components, other hooks) to also memoize. The compiler short-circuits this cascade.
16+
- Consistent adoption keeps the memoization story legible across the codebase — reviewers don't decide case-by-case whether a given `useCallback` is load-bearing or ceremonial.
17+
18+
## Rules
19+
20+
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.
21+
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.
22+
3. **Callbacks passed to JSX** (`onClick`, `onChange`, `action`, `onSubmit`, …) must be plain inline or named functions. The compiler memoizes them.
23+
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`.
24+
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.
25+
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.
26+
27+
## Pattern
28+
29+
### ✅ Correct — plain functions, `'use memo'` directive
30+
31+
```tsx
32+
const UserProfile: React.FC<Props> = ({ user, onSelect }) => {
33+
'use memo';
34+
const { t } = useTranslation();
35+
36+
const fullName = `${user.firstName} ${user.lastName}`;
37+
38+
const handleClick = () => {
39+
onSelect(user.id);
40+
};
41+
42+
return (
43+
<button onClick={handleClick}>
44+
{t('userProfile.Greet', { name: fullName })}
45+
</button>
46+
);
47+
};
48+
```
49+
50+
### ✅ Correct — custom hook with plain returned functions
51+
52+
```tsx
53+
export const useSomething = (): [Value, (next: Value) => void] => {
54+
'use memo';
55+
const [value, setValue] = useState<Value>(initial);
56+
57+
const update = (next: Value) => {
58+
setValue(next);
59+
// side effects here, reading latest closure values
60+
};
61+
62+
return [value, update];
63+
};
64+
```
65+
66+
### ❌ Wrong — manual `useCallback` wrapping a simple handler
67+
68+
```tsx
69+
const UserProfile: React.FC<Props> = ({ user, onSelect }) => {
70+
// no 'use memo'
71+
const handleClick = useCallback(() => {
72+
onSelect(user.id);
73+
}, [onSelect, user.id]);
74+
75+
return <button onClick={handleClick}>...</button>;
76+
};
77+
```
78+
79+
### ❌ Wrong — `useMemo` for trivial derivations
80+
81+
```tsx
82+
const UserProfile: React.FC<Props> = ({ user }) => {
83+
// no 'use memo'
84+
const fullName = useMemo(
85+
() => `${user.firstName} ${user.lastName}`,
86+
[user.firstName, user.lastName],
87+
);
88+
return <span>{fullName}</span>;
89+
};
90+
```
91+
92+
## Migration
93+
94+
When editing a file that contains `useMemo` or `useCallback`:
95+
96+
1. Add `'use memo'` to the enclosing component / hook body if it is not already there.
97+
2. Unwrap each `useMemo(() => expr, deps)` into `const value = expr;`.
98+
3. Unwrap each `useCallback(fn, deps)` into the original function body, as a plain `const fn = (...)` or inline in JSX.
99+
4. Drop the `useMemo` / `useCallback` import if it becomes unused.
100+
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.
101+
102+
## Verification
103+
104+
After editing a memoization-relevant file, confirm:
105+
106+
- `'use memo'` appears on the first line of the component / hook body.
107+
- No new `useMemo` or `useCallback` imports or calls were introduced.
108+
- `bash scripts/verify.sh` passes — the compiler is exercised during `pnpm run lint` and the TypeScript build.
109+
- 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.
110+
111+
## Related
112+
113+
- `use-effect-event.md` — companion rule for the `useEffectEvent` primitive used inside effects.
114+
- `CLAUDE.md` → "React Essentials" — project-wide React conventions, including the `'use memo'` requirement.

.specs/draft-stoken-login-boundary/metadata.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@
5454
{
5555
"key": "FR-2634",
5656
"title": "[1.7] CI grep rule forbidding URL APIs in STokenLoginBoundary",
57-
"prNumber": 6854,
58-
"prUrl": "https://github.com/lablup/backend.ai-webui/pull/6854"
57+
"prNumber": null,
58+
"prUrl": null,
59+
"status": "wont-do",
60+
"note": "Closed as redundant. URL-free invariant enforced by FR-2633 test assertion."
5961
},
6062
{
6163
"key": "FR-2635",
@@ -147,5 +149,5 @@
147149
},
148150
"sourceIssues": [],
149151
"createdAt": "2026-04-21T00:00:00Z",
150-
"notes": "Epic FR-2616 filed. Spec Task FR-2618 created as sub-issue under the Epic. Spec PR #6828 resolves FR-2618. Dev plan landed on same PR stack. Story 1 implementation PRs #6850-#6856 submitted (FR-2628-FR-2634 + test). FR-2635 closed as investigation-only (comment on issue). Directory still uses draft- prefix; rename to FR-2616-stoken-login-boundary as a follow-up."
152+
"notes": "Epic FR-2616 filed. Spec Task FR-2618 created. Story 1 implementation: PRs #6850-#6853, #6855-#6857 submitted (FR-2628, FR-2629, FR-2630, FR-2631, FR-2632, FR-2633, refactor). FR-2634 closed as Not Planned (duplicate of FR-2633 test assertion). FR-2635 closed as investigation-only (Jira comment). Directory still uses draft- prefix; rename to FR-2616-stoken-login-boundary as a follow-up."
151153
}

react/src/components/STokenLoginBoundary.tsx

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { BAIButton, BAICard, BAIFlex, useBAILogger } from 'backend.ai-ui';
2525
import { useAtomValue } from 'jotai';
2626
import {
2727
Suspense,
28-
useCallback,
2928
useEffect,
3029
useEffectEvent,
3130
useRef,
@@ -104,6 +103,7 @@ type Phase =
104103
export const STokenLoginBoundary: React.FC<STokenLoginBoundaryProps> = (
105104
props,
106105
) => {
106+
'use memo';
107107
return (
108108
<Suspense fallback={props.fallback ?? <DefaultFallback />}>
109109
<STokenLoginBoundaryInner {...props} />
@@ -240,10 +240,10 @@ const STokenLoginBoundaryInner: React.FC<STokenLoginBoundaryProps> = ({
240240
runLoginSequence();
241241
}, [retryKey]);
242242

243-
const retry = useCallback(() => {
243+
const retry = () => {
244244
setPhase({ name: 'pending' });
245245
setRetryKey((k) => k + 1);
246-
}, []);
246+
};
247247

248248
if (phase.name === 'error') {
249249
if (errorFallback) {
@@ -278,6 +278,7 @@ const kindToI18nKey = (kind: STokenLoginError['kind']): string =>
278278
* working but hasn't failed.
279279
*/
280280
const DefaultFallback: React.FC = () => {
281+
'use memo';
281282
const { t } = useTranslation();
282283
return (
283284
<BAIFlex
@@ -325,16 +326,15 @@ const DefaultErrorCard: React.FC<{
325326
? String((error.cause as Error)?.message ?? error.cause)
326327
: null;
327328

328-
const handleRetry = useCallback(async () => {
329-
// Wrap in a Promise so BAIButton.action triggers its async loading
330-
// state; the synchronous state reset completes before the next
331-
// render, which is visually indistinguishable from the live
332-
// sequence restart.
329+
// Wrap in a Promise so BAIButton.action triggers its async loading
330+
// state; the synchronous state reset completes before the next render,
331+
// which is visually indistinguishable from the live sequence restart.
332+
const handleRetry = async () => {
333333
await Promise.resolve();
334334
onRetry();
335-
}, [onRetry]);
335+
};
336336

337-
const handleCopy = useCallback(async () => {
337+
const handleCopy = async () => {
338338
const payload = {
339339
kind: error.kind,
340340
cause: causeDetail,
@@ -346,7 +346,7 @@ const DefaultErrorCard: React.FC<{
346346
logger.warn('[STokenLoginBoundary] clipboard write failed', copyError);
347347
message.error(t('sTokenLoginBoundary.ErrorDetailsCopyFailed'));
348348
}
349-
}, [error, causeDetail, message, t, logger]);
349+
};
350350

351351
return (
352352
<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.
@@ -53,16 +53,13 @@ export const useSToken = (): [
5353
logDeprecationIfNeeded();
5454
}, [sToken, stoken]);
5555

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

6764
const effective = sToken ?? stoken;
6865
return [effective, clear];

0 commit comments

Comments
 (0)