fix(core): make Table column utilities callable from React Server Components#3466
fix(core): make Table column utilities callable from React Server Components#3466arham766 wants to merge 2 commits into
Conversation
|
@arham766 is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
|
@josephfarina this implements the fix for #3457 whenever you have time to look. Verified end to end in example-nextjs: the exact snippet from your issue fails on main and statically prerenders with this change. |
…ponents The Table barrel (index.ts) carried a 'use client' directive, which marked the pure column utilities re-exported through it (proportional, pixel, generateColumns, paginateData) as client functions: calling them during a server render threw 'Attempted to call proportional() from the server'. Every client module in Table/ already carries its own directive, so the barrel needs none. Adds a source-invariant test pinning both halves of the boundary and documents which parts of the data-driven API are server-safe. Fixes facebook#3457
f2699fa to
23709f6
Compare
josephfarina
left a comment
There was a problem hiding this comment.
Thank you for the quick fix! Just a couple cleanliness comments
| // Copyright (c) Meta Platforms, Inc. and affiliates. | ||
|
|
||
| 'use client'; | ||
| // No 'use client' here: this file only re-exports. Each client module below |
There was a problem hiding this comment.
Just removing use client is probably okay i don't think we need the comment
| // Copyright (c) Meta Platforms, Inc. and affiliates. | ||
|
|
||
| /** | ||
| * @file Source invariants for the Table server-client boundary. |
There was a problem hiding this comment.
I'm not sure this test is high value. Just making sure it works with a test plan is probably good enough for this fix!
|
Done, both addressed: dropped the comment (plain directive removal) and removed the invariant test. The end-to-end verification stays documented in the test plan. |
josephfarina
left a comment
There was a problem hiding this comment.
Thanks for the fix!
Summary
Fixes #3457 (also filed as #3459).
proportional()andpixel()are pure factory functions returning plain{type, value}objects, but the Table barrel (Table/index.ts) carried a'use client'directive, so importing them via@astryxdesign/core/Tablehanded Server Components client references that throwAttempted to call proportional() from the serverwhen called.Every client module in
Table/already carries its own directive (Table.tsx,BaseTable.tsx, every cell/row module, every plugin hook), so the barrel needs none: with the directive removed, bundlers resolve the client boundary at the defining modules, the interactive Table stays a client island, and the pure column utilities (proportional,pixel,generateColumns,paginateData) become importable and callable from Server Components. The repo'scheck-use-client.mjsnever required a directive on this file — it only checks modules that import React client APIs, which the barrel doesn't.Also:
rscBoundary.test.ts: source-invariant test pinning both halves (barrel and pure utilities stay directive-free; component modules keep theirs), so the directive can't silently come back.Table.doc.mjs: best-practice entry (en + dense) documenting what is and isn't server-safe — column defs without function props work in RSC;renderCelland other function props still need a'use client'wrapper, since functions cannot cross the server-client boundary.Test plan
Reproduced end to end in
apps/example-nextjs(App Router, consumes the built dist) with a Server Component page using the exact pattern from the issue (data+columnswithproportional(1)/pixel(120), no'use client'):next buildfails during prerender withError: Attempted to call proportional() from the server but proportional is on the client.next buildsucceeds and the page statically prerenders (✓ Generating static pages, route listed as static).The repro page is not committed; the in-repo regression guard is
rscBoundary.test.ts.npx vitest run packages/core/src/Table/— 322/322 pass (16 files, including the new invariant test).node scripts/check-changesets.mjspasses.