Skip to content

fix(frontend): switch React Compiler to opt-in annotation mode#2375

Open
malinskibeniamin wants to merge 2 commits intomasterfrom
fix/react-compiler-opt-in-mode
Open

fix(frontend): switch React Compiler to opt-in annotation mode#2375
malinskibeniamin wants to merge 2 commits intomasterfrom
fix/react-compiler-opt-in-mode

Conversation

@malinskibeniamin
Copy link
Copy Markdown
Contributor

Summary

  • Switch React Compiler from opt-out ("infer") to opt-in (compilationMode: 'annotation') in rsbuild.config.ts
  • Remove all 71 'use no memo' directives across 58 files (now dead code — compiler only processes files with 'use memo')
  • Update regression test comment in data-table-view-options.test.tsx to reflect the new approach

Problem

The React Compiler was running in default "infer" (opt-out) mode, automatically memoizing every component and hook in src/. This caused:

  • Pagination not re-rendering when page state changes (stale closures from over-memoization)
  • Data table filters showing stale valuesdata-table-filter.tsx and related components broke under compiler memoization
  • 58 files already needed 'use no memo' workarounds to function correctly
  • Conflicts with the uiSettings Proxy pattern, URL-based state (nuqs), and TanStack Table's internal state management

Solution

Switch to compilationMode: 'annotation' — the compiler now only processes files explicitly marked with 'use memo'. No files are opted in yet. This is a stabilization change that eliminates all compiler-related memoization bugs while keeping the infrastructure in place for incremental opt-in.

What changes:

Before After
Mode Opt-out (compile everything) Opt-in (compile nothing unless marked)
'use no memo' directives 71 across 58 files 0 (removed — now dead code)
Pagination/filter bugs Present, worked around Eliminated at source

Future work:

  1. Incrementally add 'use memo' to safe presentational components after testing
  2. Address uiSettings Proxy pattern before opting in components that use it
  3. Consider panicThreshold: 'critical_errors' once confidence grows

Test plan

  • bun run type:check — passes
  • bun run lint — passes (pre-existing warnings only)
  • bun run test — 80 files, 856 tests passed, 0 failures
  • No 'use no memo' directives remain (grep -r "use no memo" src/ returns only the updated test comment)
  • Manual verification: pagination, filters, search, sorting all work in browser
  • CI green

🤖 Generated with Claude Code

The compiler was running in default "infer" (opt-out) mode, memoizing
every component/hook globally. This caused stale closures and missed
re-renders in pagination, data table filters, and search components —
58 files already needed 'use no memo' workarounds.

Switch to compilationMode: 'annotation' so the compiler only processes
files explicitly marked with 'use memo'. Remove all 71 'use no memo'
directives (now dead code). No files are opted in yet — this is a
stabilization change. Files can be incrementally opted in after testing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@malinskibeniamin malinskibeniamin self-assigned this Apr 11, 2026
@malinskibeniamin malinskibeniamin requested a review from a team April 11, 2026 18:12
@malinskibeniamin
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude bot commented Apr 11, 2026

Claude finished @malinskibeniamin's task in 1m 21s —— View job


Code Review

  • Gather context and understand the PR
  • Read key changed files
  • Review rsbuild config change
  • Review 'use no memo' removal across files
  • Review test file updates
  • Post review feedback

Overall: Clean, well-justified stabilization change. The approach is correct — 58 files needing 'use no memo' workarounds is a clear signal the opt-out mode was wrong for this codebase. The fix is minimal and the cleanup is thorough.

frontend/rsbuild.config.ts — Config change

The single-line addition is correct:

compilationMode: 'annotation',

One thing worth understanding: the sources callback (lines 35–46) continues to operate as an eligibility filter even in annotation mode. Files in /lib/redpanda-ui/, /gen/, or node_modules cannot be opted in even if someone adds 'use memo' to them — which is actually a nice safety net. This is the right behavior but it's worth making sure the team is aware: adding 'use memo' to a file in lib/redpanda-ui/ will silently have no effect.

Consider adding a comment near the sources function to document this:

// In annotation mode, this still gates which files CAN be opted in.
// Files outside these paths are ineligible even with 'use memo'.
sources: (filename: string) => { ... }

Suggest this comment →

'use no memo' removal — 58 files

All 71 directives have been correctly removed. Verified with grep -r "use no memo" src/ — the only remaining match is a comment in embedded-app.tsx ("In the future we might decide to use memo() as well") which is unrelated and fine.

Note on performance: The compiler is now effectively a no-op — no files are opted in, so zero memoization is happening via the compiler. Components that were genuinely benefiting from compiler memoization (those without 'use no memo') will lose that optimization. Given the scale of bugs caused, this trade-off is clearly correct, but it's worth tracking in the follow-up work.

data-table-view-options.test.tsx — Test comment update

The updated regression comment is accurate and well-written:

/**
 * Regression tests for DataTable search, faceted filter, and view options.
 *
 * These components broke when React Compiler memoized stale closure values
 * in data-table.tsx and data-table-filter.tsx. The fix was switching the compiler
 * to opt-in mode (compilationMode: 'annotation') and replacing
 * DropdownMenuPrimitive.Trigger with DropdownMenuTrigger.
 */

Good that it documents both root causes (mode change + primitive trigger replacement) — future engineers will appreciate this context.

Minor observations

  1. panicThreshold not yet configured: The PR mentions panicThreshold: 'critical_errors' as a future step. Consider also adding it now alongside compilationMode: 'annotation' — it's a low-risk safety net that prevents the compiler from silently breaking files during incremental opt-in.

  2. No 'use memo' files exist yet: This is intentional per the PR description. The compiler is installed but dormant. The incremental opt-in plan (start with pure presentational components) is the right approach.

  3. target: '18': Correct for React 18. When/if React 19 is adopted, this will need updating.

Summary

This is a solid fix. The 1-line config change, 128-line cleanup, and updated test comment all hang together well. The PR description is thorough and the test plan is comprehensive. No blocking issues — the suggestions above are optional improvements.

- Add comment above sources callback explaining it gates opt-in
  eligibility even in annotation mode
- Add panicThreshold: 'critical_errors' so the build fails loudly
  if the compiler can't handle an explicitly opted-in file

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@malinskibeniamin
Copy link
Copy Markdown
Contributor Author

Thanks for the review! Addressed all feedback in 621df6f:

  1. Sources comment — Added a comment above the sources callback clarifying that in annotation mode it still gates which files can be opted in, so files in lib/redpanda-ui/, gen/, or node_modules are ineligible even with 'use memo'.

  2. panicThreshold: 'critical_errors' — Added now alongside compilationMode: 'annotation'. This ensures that when files are incrementally opted in with 'use memo', the build fails loudly if the compiler encounters issues rather than silently degrading.

  3. embedded-app.tsx:79 — Verified this is a code comment about React.memo() (the React API), not the 'use memo' compiler directive. Unrelated, no action needed.

  4. Performance note — Acknowledged. Components that were benefiting from compiler memoization (those without 'use no memo') will lose that optimization. This is an acceptable trade-off given the scale of bugs, and we'll track it during incremental opt-in.

  5. target: '18' — Noted for React 19 migration.

@malinskibeniamin
Copy link
Copy Markdown
Contributor Author

@claude review

@claude
Copy link
Copy Markdown

claude bot commented Apr 12, 2026

Claude finished @malinskibeniamin's task in 53s —— View job


Code Review

  • Gather context and understand the PR
  • Read rsbuild.config.ts to verify feedback was addressed
  • Read data-table-view-options.test.tsx
  • Check 'use no memo' removal (grep confirms zero remain)
  • Post review feedback

All previous feedback has been correctly addressed. Here's the verification:

frontend/rsbuild.config.ts (lines 34–49) — Verified ✓

Both follow-up items from the previous review are in place:

compilationMode: 'annotation',
panicThreshold: 'critical_errors',
// In annotation mode, this still gates which files CAN be opted in.
// Files excluded here are ineligible even with 'use memo'.
sources: (filename: string) => { ... }
  • panicThreshold: 'critical_errors' added correctly at line 35 — ensures build fails loudly if the compiler encounters issues during incremental opt-in, rather than silently degrading.
  • The sources comment at lines 36–37 is accurate and concise. It tells the reader why the callback matters in annotation mode, not just what it does.

'use no memo' removal — Verified ✓

grep -r "use no memo" src/ returns zero matches. The only hit for "use memo" in src/ is the unrelated prose comment in embedded-app.tsx:79 ("In the future we might decide to use memo() as well"), which is about the React.memo() API, not the compiler directive.

data-table-view-options.test.tsx (lines 33–40) — Verified ✓

Regression comment accurately documents both root causes:

  • Switching the compiler to opt-in mode
  • Replacing DropdownMenuPrimitive.Trigger with DropdownMenuTrigger

Good that both are captured — this is exactly the kind of context future engineers need when debugging a resurfacing issue.

Summary

This PR is clean and production-ready. The config change is a 3-property block (compilationMode, panicThreshold, sources with its comment), the directive cleanup is complete, and the test comment is thorough. No blocking issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant