Skip to content

fix(rsc): validate client-only/server-only imports after tree-shaking - #1511

Closed
james-elicx wants to merge 2 commits into
vitejs:mainfrom
james-elicx:fix/rsc-validate-imports-tree-shaking
Closed

james-elicx wants to merge 2 commits into
vitejs:mainfrom
james-elicx:fix/rsc-validate-imports-tree-shaking

Conversation

@james-elicx

@james-elicx james-elicx commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

rsc:validate-imports checked the client-only / server-only import chain in buildEnd, which runs before tree-shaking. A server component importing one export from a sideEffects: false barrel (e.g. import { Toast } from "@heroui/react") failed the build when an unused leaf of that barrel imported client-only, even though the bundler drops the leaf and the marker from the output:

[plugin rsc:validate-imports] .../react-aria-components/dist/exports/Calendar.mjs
Error: 'client-only' cannot be imported in server build ('rsc' environment):
 imported by node_modules/react-aria-components/dist/exports/Calendar.mjs
  imported by node_modules/@heroui/react/dist/components/calendar-year-picker/use-calendar-state.js
   imported by node_modules/@heroui/react/dist/components/calendar-year-picker/index.js
    imported by node_modules/@heroui/react/dist/index.js
     imported by app/page.tsx

The same happened when a module imports client-only but only re-exports a used binding (react-aria-components exports/Toast.mjs, in a package with "sideEffects": ["*.css"]), and in the mirror case of server-only reached from a "use client" module. Reported in cloudflare/vinext#4007.

Fix

  • The build check now runs in generateBundle and fails only when the invalid marker module, which has side effects, is in an emitted chunk's moduleIds, i.e. when it actually ships. Direct imports, CJS require, dynamic import(), used leaves, and packages without sideEffects: false still fail.
  • The import chain starts from the nearest module kept in the output (searching static and dynamic importers), so the error names the import that kept the marker rather than a dropped leaf.
  • Rollup (Vite 7) and Rolldown differ on one shape: Rollup keeps the side-effect imports of a module on a re-export path when the re-exported binding is used, so for the Toast shape the marker really ships with Rollup and the build still fails there. The check follows what each bundler emits.
  • The scan build is skipped because it strips modules down to their imports; the real build does the check.
  • Dev keeps the existing transform-time check because Vite dev doesn't tree-shake. The error message format is unchanged.

This matches Next.js with Turbopack, which reports client-only / server-only import errors only for modules that remain in the tree-shaken graph (insert_client_only_error_alias / insert_server_only_error_alias in crates/next-core/src/next_import_map.rs). Next.js with webpack still rejects these barrels.

Tests

New cases in e2e/validate-imports.test.ts, using fixture packages that mirror the heroui → react-aria-components graph:

  • builds and renders: an unused client-only leaf of a sideEffects: false barrel; the Toast re-export shape in a package with a glob sideEffects array (Rolldown only); an unused server-only leaf reached from a "use client" component (all fail on main)
  • still fails: a used leaf (full chain asserted), a package without sideEffects: false, a CJS require, a dynamic import() whose chain names the retained importer (names the dropped leaf on main), a used server-only leaf from a client component, and on Rollup the Toast shape, whose chain names the re-export that kept the marker rather than the dropped calendar.js (names calendar.js on main)
  • verified locally on Vite 8 and on Vite 7 (vite: ^7, @vitejs/plugin-react: ^5 overrides, as in CI)

Trade-offs for reviewers

  • A build with a genuinely invalid import now reports the error after the real build of the offending environment (step 3/5 for client-only, 4/5 for server-only) instead of during the first scan build. On the reporter's app (~250 modules per environment) that is ~1.1 s instead of ~0.4 s to the error; on larger apps the gap is larger.
  • Successful builds are unaffected: the hook collects the output's module ids into a set, ~0.05–0.08 ms per environment on the reporter's app (median total build time 1.94 s before and after, 5 runs each).
  • When the build fails, finding the chain walks up the importer graph from the marker to the nearest module kept in the output, so its cost is bounded by the modules that transitively import the marker. It took ~0.1 ms on the reporter's app with a direct invalid import, and only runs on the error path.
  • Dev still rejects these barrels. On the reporter's app, dev also fails independently of this check, because evaluating the whole barrel in the RSC environment runs react-aria's top-level createContext. Addressing dev would need barrel/re-export optimization in dev, which is out of scope here.

@hi-ogawa

Copy link
Copy Markdown
Contributor
  • Dev still rejects these barrels. On the reporter's app, dev also fails independently of this check, because evaluating the whole barrel in the RSC environment runs react-aria's top-level createContext. Addressing dev would need barrel/re-export optimization in dev, which is out of scope here.

Thanks for the PR and write-up. The dev still failing is a concern. Rather than partially fixing for build, I'm wondering if we can recommend configuring barrel transform plugin manually as a stopgap. The workaround looks like this #1519

@james-elicx

Copy link
Copy Markdown
Contributor Author

Alrighty, so I've gone back to my agents and reworked it into our existing plugin for import optimisation. And it works decently without much of a perf impact.

also funnily enough i found that webpack actually has this same issue while turbopack doesn't. See the below table my agent created:

Server Component imports Next webpack Next Turbopack
{ Toast } from @heroui/react, rendering <Toast.Provider /> client-only error renders
{ Skeleton } from @heroui/react client-only error (renders with optimizePackageImports) renders
{ UNSTABLE_ToastQueue } from react-aria-components/Toast client-only error renders
{ Collection } from react-aria-components/Collection client-only error createContext is not a function

from what i gather, turbopack will follow the re-exports without running the client-only import, or something like that. We already had a plugin vinext for Next.js' optimizePackageImports stuff, and it does our own barrel file rewrites instead, so i've tried to replicate that behaviour in cloudflare/vinext#4099 instead.

for what it's worth, i do think this pr highlights a genuine bug that should be fixed, but i can appreciate that it may not be ideal to fix the build side without fixing the dev side, so i don't mind if you want to close it.


My agent's summary:

  • Vinext already implements Next's optimizePackageImports, so Next apps expect barrel imports to work without extra plugin config. @heroui/react is now on its default list.
  • It rewrites from the barrel's actual re-exports instead of name → path templates, so names that don't follow a naming convention (cn, toast, skeletonVariants) need no per-package rules.
  • It covers Toast, which subpath imports in app code can't fix: heroui's own toast-queue.js imports react-aria-components/Toast, itself a barrel that reaches module-scope createContext and client-only. In the RSC environment only, vinext also rewrites react-aria-components subpath imports inside the dependency, resolving them from the importing package because react-aria-components ships its own nested copy of react-stately. That gives Turbopack's results above in vinext dev and build.

@hi-ogawa

Copy link
Copy Markdown
Contributor

from what i gather, turbopack will follow the re-exports without running the client-only import, or something like that

Oh, this looks more involved than I thought initially. For this behavior though, to be honest, this looks like turbopack is bending client-only semantics since that basically makes react-aria-component/Toast's client-only being treated as no-op while the package side intent is exactly that https://github.com/adobe/react-spectrum/blob/797762ed3526ea12f089e321bd3891066affad70/packages/react-aria-components/exports/Toast.ts#L13-L15 So I feel this specific issue's fix belongs to the package side @heroui/react and react-aria-component/Toast.

For posterity, some quick sketch of how import chain look likes https://gist.github.com/hi-ogawa/ff662a548d746ed213ea441467bbeddc#file-0-toast-md

@hi-ogawa

Copy link
Copy Markdown
Contributor

for what it's worth, i do think this pr highlights a genuine bug that should be fixed, but i can appreciate that it may not be ideal to fix the build side without fixing the dev side, so i don't mind if you want to close it.

So, looking at again, I was probably mislead by react-aria-component/Toast example, but what this PR fixes with check-after-tree-shaking approach looks sound. I think we'll revisit when dev/build consistency becomes more viable with dev bundled mode.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants