vite-plugin-react PR 1234 export-star review notes

pr-1234-export-star-review.md

PR 1234 export-star review notes

Context

PR: vitejs/vite-plugin-react#1234

The PR handles bare export * from "..." in RSC boundary modules by pre-expanding star re-exports before the existing proxy/wrap transforms run.

Main implementation:

• collectExportNames: packages/plugin-rsc/src/plugin.ts#L1408
• expandExportAllDeclarations: packages/plugin-rsc/src/plugin.ts#L1479
• use client hook-up: packages/plugin-rsc/src/plugin.ts#L1583
• use server hook-up: packages/plugin-rsc/src/plugin.ts#L2068

Main finding

The main issue is architectural: the export-star expansion is implemented directly inside plugin.ts, mixed with Vite/Rollup resolution, filesystem reads, source normalization, recursive export scanning, merge policy, and MagicString rewriting.

That makes the new behavior hard to unit test at the right level. The PR therefore only adds end-to-end coverage for one happy use client path, while the same expander is also wired into use server handling. The high-value next step is to pull the expansion into a transform-adjacent helper under packages/plugin-rsc/src/transforms/ with injected resolve/load/parse hooks, then unit-test the helper directly and keep plugin.ts as the adapter.

The ESM edge cases below are supporting evidence for why this needs a focused test seam, not necessarily the main review headline.

Behavior risks

The current expansion rewrites each bare star independently into explicit named re-exports:

export * from "./a"

becomes:

export { x, y } from "./a"

This can diverge from the practical RSC export-name surface in important cases:

• Explicit names in the current module should win over star-derived names.
• Names contributed by multiple bare star sources should be dropped as ambiguous unless explicitly exported by the current module.
• default should never be contributed by a bare export *.
• Scan failures should not be treated as an empty export list.

Concrete risk: without unit tests around the expander, a valid barrel with duplicate star names can be converted into an unconditional duplicate-export syntax error, or a local explicit export can conflict with a rewritten star export.

Current divergences

Current collector behavior:

• Collects names into a flat array: packages/plugin-rsc/src/plugin.ts#L1438
• Recursively appends star target names without ambiguity tracking: packages/plugin-rsc/src/plugin.ts#L1462
• Rewrites each star with the raw name array: packages/plugin-rsc/src/plugin.ts#L1503
• Removes the star export when the collected list is empty: packages/plugin-rsc/src/plugin.ts#L1500

Unsupported or lossy cases:

• export * from "./a"; export * from "./b" where both export x
• export * from "./a"; export const x = 1
• unresolved, virtual, unreadable, or unparsable targets
• arbitrary string export names such as export { x as "my thing" }
• export * as ns from "./a" in transformWrapExport is questionable for current callers: for top-level use server, ns is a namespace object rather than an async server function; for RSC CSS export wrapping, default filtering leaves it unwrapped but still causes import/export rewrite and helper churn.

Suggested behavior target

For the RSC proxy/wrap use case, a pragmatic target is:

• Build a module-level export surface, not one replacement per star in isolation.
• Track explicit exports from the current module.
• Track names contributed by each bare star source.
• Emit only explicit names plus star names that are unique across star sources and not shadowed by explicit names.
• Omit ambiguous star names.
• Omit default from bare star expansion.
• Hard-error on scan failures or unsupported export names first, since this code already errored before the PR.

Hard-error-first is preferable to silent deletion. It can be loosened later once the supported behavior is clear.

Possible shape

Move the expander under packages/plugin-rsc/src/transforms/ as a transform-adjacent helper with injected IO. The purpose of the shape is to create a focused unit-test seam; the plugin should only provide Vite/Rollup adapters:

type StarExportSource = {
  node: ExportAllDeclaration
  source: string
  resolvedId: string
  names: Set<string>
}

type ModuleExportScan = {
  explicitNames: Set<string>
  starSources: StarExportSource[]
}

type StarRewritePlan = {
  node: ExportAllDeclaration
  source: string
  names: string[]
}

async function transformExpandExportAll(
  code: string,
  ast: Program,
  options: {
    importer: string
    resolve: (source: string, importer: string) => Promise<string | undefined>
    load: (id: string) => Promise<string | undefined>
    parse: (code: string) => Promise<Program>
  },
): Promise<{ code: string; ast: Program } | undefined>

async function scanCurrentModule(
  ast: Program,
  bareStars: ExportAllDeclaration[],
  options: {
    importer: string
    resolve: (source: string, importer: string) => Promise<string | undefined>
    load: (id: string) => Promise<string | undefined>
    parse: (code: string) => Promise<Program>
    cache?: Map<string, Promise<Set<string>>>
  },
): Promise<ModuleExportScan>

function buildStarRewritePlan(scan: ModuleExportScan): StarRewritePlan[]

The plugin layer should adapt Vite/Rollup APIs. The helper should own export-name merge policy and be unit-testable.

Rough routine:

async function transformExpandExportAll(code, ast, options) {
  const bareStars = findBareExportAllDeclarations(ast)
  if (bareStars.length === 0) return

  const scan = await scanCurrentModule(ast, bareStars, options)
  const plan = buildStarRewritePlan(scan)

  const output = new MagicString(code)
  for (const item of plan) {
    if (item.names.length === 0) {
      // Start stricter and hard-error first. If this is loosened later,
      // side-effect import is safer than removal for server-to-server output.
      throw new Error("export * does not expose any supported unambiguous names")
    }
    output.update(
      item.node.start,
      item.node.end,
      `export { ${item.names.join(", ")} } from ${JSON.stringify(item.source)};`,
    )
  }

  const newCode = output.toString()
  return { code: newCode, ast: await options.parse(newCode) }
}

scanCurrentModule should:

• collect explicit names from the current AST, including local exports, named re-exports, and export * as ns;
• reject unsupported export names for now, such as string-literal names;
• resolve/load/parse each bare star target;
• recursively collect the target's exported names;
• cache by resolved id before recursion so cycles terminate.

buildStarRewritePlan should:

• count star-contributed names across all star sources;
• omit default;
• omit names already present in explicitNames;
• omit names contributed by more than one star source;
• return per-star replacement items so each original export * from "..." can be rewritten with only that source's allowed names.

Coverage gaps

The new e2e fixture only covers a use client star barrel:

• Fixture: packages/plugin-rsc/examples/basic/src/routes/export-all/index.tsx#L1
• Test: packages/plugin-rsc/e2e/basic.test.ts#L1877

Since the PR also hooks into vitePluginUseServer, add integration coverage for top-level use server with export *:

• server-to-server: real server module path through transformServerActionServer
• server-to-client: proxy path through transformDirectiveProxyExport

Also add focused transform/helper tests for:

• duplicate star names are omitted
• explicit export wins over star export
• default from a star target is omitted
• unresolved/unreadable/unparsable target hard-errors
• arbitrary string export names hard-error or are explicitly documented unsupported
• export * as ns remains unsupported or caller-gated in transformWrapExport unless a concrete wrap use case exists

Prior art

Vitest native mocker has a similar architecture: recursively parse export names, resolve reexport edges, and feed concrete names into source generation.

Repo: https://github.com/vitest-dev/vitest

• packages/mocker/src/node/parsers.ts: collectModuleExports, ESM star recursion via es-module-lexer, and CJS reexport recursion via cjs-module-lexer.
• packages/mocker/src/node/automock.ts: direct automock star handling.
• Vitest resolves ESM reexports with import.meta.resolve(source, parentFileUrl) and CJS reexports with createRequire(filename).resolve(source).

Node prior art for CJS-to-ESM named exports:

Repo: https://github.com/nodejs/node

• lib/internal/modules/esm/translators.js: uses internalBinding("cjs_lexer"), pre-parses exports before evaluation, caches before recursion to handle cycles, and recurses through detected CJS reexports.
• doc/api/esm.md: documents CJS named export detection as heuristic and static.

Node/V8 prior art for actual ESM star semantics:

• deps/v8/src/objects/source-text-module.cc: star export resolution checks multiple star providers for the same name; namespace construction marks conflicting star names ambiguous and skips them.