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
114 changes: 114 additions & 0 deletions .claude/rules/react-compiler-memoization.md
Original file line number Diff line number Diff line change
@@ -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<Props> = ({ user, onSelect }) => {
'use memo';
const { t } = useTranslation();

const fullName = `${user.firstName} ${user.lastName}`;

const handleClick = () => {
onSelect(user.id);
};

return (
<button onClick={handleClick}>
{t('userProfile.Greet', { name: fullName })}
</button>
);
};
```

### ✅ Correct — custom hook with plain returned functions

```tsx
export const useSomething = (): [Value, (next: Value) => void] => {
'use memo';
const [value, setValue] = useState<Value>(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<Props> = ({ user, onSelect }) => {
// no 'use memo'
const handleClick = useCallback(() => {
onSelect(user.id);
}, [onSelect, user.id]);

return <button onClick={handleClick}>...</button>;
};
```

### ❌ Wrong — `useMemo` for trivial derivations

```tsx
const UserProfile: React.FC<Props> = ({ user }) => {
// no 'use memo'
const fullName = useMemo(
() => `${user.firstName} ${user.lastName}`,
[user.firstName, user.lastName],
);
return <span>{fullName}</span>;
};
```

## 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.
8 changes: 5 additions & 3 deletions .specs/draft-stoken-login-boundary/metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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."
}
22 changes: 11 additions & 11 deletions react/src/components/STokenLoginBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import { BAIButton, BAICard, BAIFlex, useBAILogger } from 'backend.ai-ui';
import { useAtomValue } from 'jotai';
import {
Suspense,
useCallback,
useEffect,
useEffectEvent,
useRef,
Expand Down Expand Up @@ -104,6 +103,7 @@ type Phase =
export const STokenLoginBoundary: React.FC<STokenLoginBoundaryProps> = (
props,
) => {
'use memo';
return (
<Suspense fallback={props.fallback ?? <DefaultFallback />}>
<STokenLoginBoundaryInner {...props} />
Expand Down Expand Up @@ -240,10 +240,10 @@ const STokenLoginBoundaryInner: React.FC<STokenLoginBoundaryProps> = ({
runLoginSequence();
}, [retryKey]);

const retry = useCallback(() => {
const retry = () => {
setPhase({ name: 'pending' });
setRetryKey((k) => k + 1);
}, []);
};

if (phase.name === 'error') {
if (errorFallback) {
Expand Down Expand Up @@ -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 (
<BAIFlex
Expand Down Expand Up @@ -325,16 +326,15 @@ const DefaultErrorCard: React.FC<{
? String((error.cause as Error)?.message ?? error.cause)
: null;

const handleRetry = useCallback(async () => {
// 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,
Expand All @@ -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]);
};
Comment thread
nowgnuesLee marked this conversation as resolved.

return (
<BAIFlex
Expand Down
19 changes: 8 additions & 11 deletions react/src/hooks/useSToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
import { useBAILogger } from 'backend.ai-ui';
import { parseAsString, useQueryState } from 'nuqs';
import { useCallback, useEffect, useEffectEvent, useRef } from 'react';
import { useEffect, useEffectEvent, useRef } from 'react';

/**
* Read and clear the sToken URL query parameter using nuqs.
Expand Down Expand Up @@ -53,16 +53,13 @@ export const useSToken = (): [
logDeprecationIfNeeded();
}, [sToken, stoken]);

const clear = useCallback(
async (next: string | null) => {
// 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];
Expand Down
Loading