diff --git a/.claude/rules/react-compiler-memoization.md b/.claude/rules/react-compiler-memoization.md new file mode 100644 index 0000000000..2faade35f9 --- /dev/null +++ b/.claude/rules/react-compiler-memoization.md @@ -0,0 +1,114 @@ +--- +description: Rely on the React Compiler (`'use memo'`) instead of manual `useMemo` / `useCallback` +paths: + - "react/**/*.{tsx,ts}" + - "packages/backend.ai-ui/**/*.{tsx,ts}" +--- + +# React Compiler Memoization Rule + +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. + +## Why + +- Manual `useMemo` / `useCallback` dependency arrays drift out of sync as code evolves; the compiler's view is always current with the code it compiles. +- `useCallback` forces every identity-sensitive caller (child components, other hooks) to also memoize. The compiler short-circuits this cascade. +- 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. + +## Rules + +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. +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. +3. **Callbacks passed to JSX** (`onClick`, `onChange`, `action`, `onSubmit`, …) must be plain inline or named functions. The compiler memoizes them. +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`. +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. +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. + +## Pattern + +### ✅ Correct — plain functions, `'use memo'` directive + +```tsx +const UserProfile: React.FC = ({ user, onSelect }) => { + 'use memo'; + const { t } = useTranslation(); + + const fullName = `${user.firstName} ${user.lastName}`; + + const handleClick = () => { + onSelect(user.id); + }; + + return ( + + ); +}; +``` + +### ✅ Correct — custom hook with plain returned functions + +```tsx +export const useSomething = (): [Value, (next: Value) => void] => { + 'use memo'; + const [value, setValue] = useState(initial); + + const update = (next: Value) => { + setValue(next); + // side effects here, reading latest closure values + }; + + return [value, update]; +}; +``` + +### ❌ Wrong — manual `useCallback` wrapping a simple handler + +```tsx +const UserProfile: React.FC = ({ user, onSelect }) => { + // no 'use memo' + const handleClick = useCallback(() => { + onSelect(user.id); + }, [onSelect, user.id]); + + return ; +}; +``` + +### ❌ Wrong — `useMemo` for trivial derivations + +```tsx +const UserProfile: React.FC = ({ user }) => { + // no 'use memo' + const fullName = useMemo( + () => `${user.firstName} ${user.lastName}`, + [user.firstName, user.lastName], + ); + return {fullName}; +}; +``` + +## Migration + +When editing a file that contains `useMemo` or `useCallback`: + +1. Add `'use memo'` to the enclosing component / hook body if it is not already there. +2. Unwrap each `useMemo(() => expr, deps)` into `const value = expr;`. +3. Unwrap each `useCallback(fn, deps)` into the original function body, as a plain `const fn = (...)` or inline in JSX. +4. Drop the `useMemo` / `useCallback` import if it becomes unused. +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. + +## Verification + +After editing a memoization-relevant file, confirm: + +- `'use memo'` appears on the first line of the component / hook body. +- No new `useMemo` or `useCallback` imports or calls were introduced. +- `bash scripts/verify.sh` passes — the compiler is exercised during `pnpm run lint` and the TypeScript build. +- 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. + +## Related + +- `use-effect-event.md` — companion rule for the `useEffectEvent` primitive used inside effects. +- `CLAUDE.md` → "React Essentials" — project-wide React conventions, including the `'use memo'` requirement. diff --git a/.specs/draft-stoken-login-boundary/metadata.json b/.specs/draft-stoken-login-boundary/metadata.json index 4b8e69b724..071b6dd2f6 100644 --- a/.specs/draft-stoken-login-boundary/metadata.json +++ b/.specs/draft-stoken-login-boundary/metadata.json @@ -54,8 +54,10 @@ { "key": "FR-2634", "title": "[1.7] CI grep rule forbidding URL APIs in STokenLoginBoundary", - "prNumber": 6854, - "prUrl": "https://github.com/lablup/backend.ai-webui/pull/6854" + "prNumber": null, + "prUrl": null, + "status": "wont-do", + "note": "Closed as redundant. URL-free invariant enforced by FR-2633 test assertion." }, { "key": "FR-2635", @@ -147,5 +149,5 @@ }, "sourceIssues": [], "createdAt": "2026-04-21T00:00:00Z", - "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." + "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." } diff --git a/react/src/components/STokenLoginBoundary.tsx b/react/src/components/STokenLoginBoundary.tsx index 24cfea1a8a..a8e89714e2 100644 --- a/react/src/components/STokenLoginBoundary.tsx +++ b/react/src/components/STokenLoginBoundary.tsx @@ -25,7 +25,6 @@ import { BAIButton, BAICard, BAIFlex, useBAILogger } from 'backend.ai-ui'; import { useAtomValue } from 'jotai'; import { Suspense, - useCallback, useEffect, useEffectEvent, useRef, @@ -104,6 +103,7 @@ type Phase = export const STokenLoginBoundary: React.FC = ( props, ) => { + 'use memo'; return ( }> @@ -240,10 +240,10 @@ const STokenLoginBoundaryInner: React.FC = ({ runLoginSequence(); }, [retryKey]); - const retry = useCallback(() => { + const retry = () => { setPhase({ name: 'pending' }); setRetryKey((k) => k + 1); - }, []); + }; if (phase.name === 'error') { if (errorFallback) { @@ -278,6 +278,7 @@ const kindToI18nKey = (kind: STokenLoginError['kind']): string => * working but hasn't failed. */ const DefaultFallback: React.FC = () => { + 'use memo'; const { t } = useTranslation(); return ( { - // Wrap in a Promise so BAIButton.action triggers its async loading - // state; the synchronous state reset completes before the next - // render, which is visually indistinguishable from the live - // sequence restart. + // Wrap in a Promise so BAIButton.action triggers its async loading + // state; the synchronous state reset completes before the next render, + // which is visually indistinguishable from the live sequence restart. + const handleRetry = async () => { await Promise.resolve(); onRetry(); - }, [onRetry]); + }; - const handleCopy = useCallback(async () => { + const handleCopy = async () => { const payload = { kind: error.kind, cause: causeDetail, @@ -346,7 +346,7 @@ const DefaultErrorCard: React.FC<{ logger.warn('[STokenLoginBoundary] clipboard write failed', copyError); message.error(t('sTokenLoginBoundary.ErrorDetailsCopyFailed')); } - }, [error, causeDetail, message, t, logger]); + }; return ( { - // Clear both keys; callers typically pass `null` after a successful - // login to strip the token from the URL. Return the final search - // params from the last setter to match nuqs' Promise contract. - await setSToken(next); - return setStoken(null); - }, - [setSToken, setStoken], - ); + // React Compiler ('use memo') memoizes the returned tuple; no manual + // useCallback wrapper is required. Callers typically pass `null` after a + // successful login to strip the token from the URL. + const clear = async (next: string | null) => { + await setSToken(next); + return setStoken(null); + }; const effective = sToken ?? stoken; return [effective, clear];