-
-
Notifications
You must be signed in to change notification settings - Fork 250
fix(rsc): handle export * re-exports in use client and use server modules
#1234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7cc932c
bc48c4d
87fd798
d72cae8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| 'use client' | ||
|
|
||
| export * from './named' | ||
|
|
||
| export function ExportAllNamed() { | ||
| return <span data-testid="export-all-named">export-all-named</span> | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| 'use client' | ||
|
|
||
| export function ExportAllA() { | ||
| return <span data-testid="export-all-a">export-all-a</span> | ||
| } | ||
|
|
||
| export function ExportAllB() { | ||
| return <span data-testid="export-all-b">export-all-b</span> | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { ExportAllA, ExportAllB, ExportAllNamed } from '.' | ||
|
|
||
| export function TestExportAll() { | ||
| return ( | ||
| <div data-testid="test-export-all"> | ||
| test-export-all: | ||
| <ExportAllA />|<ExportAllB />|<ExportAllNamed /> | ||
| </div> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,7 @@ import { | |
| } from './plugins/vite-utils' | ||
| import { | ||
| type TransformWrapExportFilter, | ||
| extractNames, | ||
| hasDirective, | ||
| transformDirectiveProxyExport, | ||
| transformServerActionServer, | ||
|
|
@@ -1375,6 +1376,143 @@ function globalAsyncLocalStoragePlugin(): Plugin[] { | |
| ] | ||
| } | ||
|
|
||
| // Strip TS/JSX so `parseAstAsync` can read the result. Prefer oxc when | ||
| // available (Vite 8+); fall back to esbuild for older Vite versions. | ||
| async function transformSourceForExportScan( | ||
| code: string, | ||
| filename: string, | ||
| ): Promise<string | undefined> { | ||
| const v = vite as Partial<{ | ||
| transformWithOxc: ( | ||
| code: string, | ||
| filename: string, | ||
| options?: { sourcemap?: boolean }, | ||
| ) => Promise<{ code: string }> | ||
| transformWithEsbuild: ( | ||
| code: string, | ||
| filename: string, | ||
| options?: { sourcemap?: boolean }, | ||
| ) => Promise<{ code: string }> | ||
| }> | ||
| const transform = v.transformWithOxc ?? v.transformWithEsbuild | ||
| if (!transform) return undefined | ||
| const result = await transform(code, filename, { sourcemap: false }) | ||
| return result.code | ||
| } | ||
|
|
||
| // Recursively collect the named exports of a module (following `export * from` | ||
| // chains), so that the RSC `use client`/`use server` proxy transforms can | ||
| // expand bare `export *` re-exports into explicit named re-exports before | ||
| // proxy generation. The pure proxy transform cannot do this on its own because | ||
| // the names live in another file. | ||
| async function collectExportNames( | ||
| ctx: Rollup.TransformPluginContext, | ||
| resolvedId: string, | ||
| seen: Set<string>, | ||
| ): Promise<string[]> { | ||
| if (seen.has(resolvedId)) return [] | ||
| seen.add(resolvedId) | ||
|
|
||
| // Read the source from disk and strip TS/JSX so the AST walk below sees | ||
| // standard ESM exports. We don't go through `this.load` / | ||
| // `transformRequest` here — in dev they return module-runner output | ||
| // (`__vite_ssr_exportName__(...)`) the walker can't read, and on build | ||
| // there's no practical benefit over reading the source directly for the | ||
| // simple TS/JSX modules we care about. | ||
| let moduleCode: string | undefined | ||
| try { | ||
| const raw = await fs.promises.readFile(resolvedId, 'utf-8') | ||
| moduleCode = await transformSourceForExportScan(raw, resolvedId) | ||
| } catch { | ||
| return [] | ||
| } | ||
| if (!moduleCode) return [] | ||
|
|
||
| let ast: Awaited<ReturnType<typeof parseAstAsync>> | ||
| try { | ||
| ast = await parseAstAsync(moduleCode) | ||
| } catch { | ||
| return [] | ||
| } | ||
|
Comment on lines
+1423
to
+1436
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can start from surfacing these errors to start with so it's easier to iterate on unsupported case. |
||
|
|
||
| const names: string[] = [] | ||
| for (const node of ast.body) { | ||
| if (node.type === 'ExportNamedDeclaration') { | ||
| if (node.declaration) { | ||
| if ( | ||
| node.declaration.type === 'FunctionDeclaration' || | ||
| node.declaration.type === 'ClassDeclaration' | ||
| ) { | ||
| if (node.declaration.id) names.push(node.declaration.id.name) | ||
| } else if (node.declaration.type === 'VariableDeclaration') { | ||
| for (const decl of node.declaration.declarations) { | ||
| names.push(...extractNames(decl.id)) | ||
| } | ||
| } | ||
| } else { | ||
| for (const spec of node.specifiers) { | ||
| if ( | ||
| spec.exported.type === 'Identifier' && | ||
| spec.exported.name !== 'default' | ||
| ) { | ||
| names.push(spec.exported.name) | ||
| } | ||
| } | ||
| } | ||
| } else if (node.type === 'ExportAllDeclaration') { | ||
| if (node.exported?.type === 'Identifier') { | ||
| names.push(node.exported.name) | ||
| } else if (node.source) { | ||
| const subResolved = await ctx.resolve( | ||
| node.source.value as string, | ||
| resolvedId, | ||
| ) | ||
| if (subResolved) { | ||
| names.push(...(await collectExportNames(ctx, subResolved.id, seen))) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return names | ||
| } | ||
|
|
||
| async function expandExportAllDeclarations( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably it's better to extract this as |
||
| ctx: Rollup.TransformPluginContext, | ||
| ast: Awaited<ReturnType<typeof parseAstAsync>>, | ||
| code: string, | ||
| id: string, | ||
| ): Promise<{ | ||
| code: string | ||
| ast: Awaited<ReturnType<typeof parseAstAsync>> | ||
| } | null> { | ||
| const targets = ast.body.filter( | ||
| (n) => n.type === 'ExportAllDeclaration' && !n.exported, | ||
| ) | ||
| if (targets.length === 0) return null | ||
|
|
||
| const output = new MagicString(code) | ||
| for (const node of targets) { | ||
| if (node.type !== 'ExportAllDeclaration') continue | ||
| const source = node.source.value as string | ||
| const resolved = await ctx.resolve(source, id) | ||
| if (!resolved) continue | ||
| const names = await collectExportNames(ctx, resolved.id, new Set()) | ||
| if (names.length === 0) { | ||
| output.remove(node.start, node.end) | ||
| } else { | ||
| output.update( | ||
| node.start, | ||
| node.end, | ||
| `export { ${names.join(', ')} } from ${JSON.stringify(source)};`, | ||
| ) | ||
| } | ||
| } | ||
| if (!output.hasChanged()) return null | ||
| const newCode = output.toString() | ||
| const newAst = await parseAstAsync(newCode) | ||
| return { code: newCode, ast: newAst } | ||
| } | ||
|
|
||
| function vitePluginUseClient( | ||
| useClientPluginOptions: Pick< | ||
| RscPluginOptions, | ||
|
|
@@ -1426,7 +1564,7 @@ function vitePluginUseClient( | |
| return | ||
| } | ||
|
|
||
| const ast = await parseAstAsync(code) | ||
| let ast = await parseAstAsync(code) | ||
| if (!hasDirective(ast.body, 'use client')) { | ||
| delete manager.clientReferenceMetaMap[id] | ||
| return | ||
|
|
@@ -1442,6 +1580,17 @@ function vitePluginUseClient( | |
| } | ||
| } | ||
|
|
||
| const expanded = await expandExportAllDeclarations( | ||
| this, | ||
| ast, | ||
| code, | ||
| id, | ||
| ) | ||
| if (expanded) { | ||
| code = expanded.code | ||
| ast = expanded.ast | ||
| } | ||
|
|
||
| let importId: string | ||
| let referenceKey: string | ||
| const packageSource = packageSources.get(id) | ||
|
|
@@ -1914,7 +2063,19 @@ function vitePluginUseServer( | |
| delete manager.serverReferenceMetaMap[id] | ||
| return | ||
| } | ||
| const ast = await parseAstAsync(code) | ||
| let ast = await parseAstAsync(code) | ||
| if (hasDirective(ast.body, 'use server')) { | ||
| const expanded = await expandExportAllDeclarations( | ||
| this, | ||
| ast, | ||
| code, | ||
| id, | ||
| ) | ||
| if (expanded) { | ||
| code = expanded.code | ||
| ast = expanded.ast | ||
| } | ||
| } | ||
|
|
||
| let normalizedId_: string | undefined | ||
| const getNormalizedId = () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,18 +168,25 @@ export function transformWrapExport( | |
| } | ||
|
|
||
| /** | ||
| * export * as ns from './foo' | ||
| * export * from './foo' | ||
| */ | ||
| // vue sfc uses ExportAllDeclaration to re-export setup script. | ||
| // for now we just give an option to not throw for this case. | ||
| // https://github.com/vitejs/vite-plugin-vue/blob/30a97c1ddbdfb0e23b7dc14a1d2fb609668b9987/packages/plugin-vue/src/main.ts#L372 | ||
| if ( | ||
| !options.ignoreExportAllDeclaration && | ||
| node.type === 'ExportAllDeclaration' | ||
| ) { | ||
| throw Object.assign(new Error('unsupported ExportAllDeclaration'), { | ||
| pos: node.start, | ||
| }) | ||
| if (node.type === 'ExportAllDeclaration') { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also, this changes looks unneeded as it cannot be useful for server function case at least. |
||
| if (node.exported?.type === 'Identifier') { | ||
| tinyassert(node.source.type === 'Literal') | ||
| const exportedName = node.exported.name | ||
| const localName = `$$import_${exportedName}` | ||
| output.remove(node.start, node.end) | ||
| toAppend.push(`import * as ${localName} from ${node.source.raw}`) | ||
| wrapExport(localName, exportedName) | ||
| } else if (!options.ignoreExportAllDeclaration) { | ||
| throw Object.assign(new Error('unsupported ExportAllDeclaration'), { | ||
| pos: node.start, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coverage for
"use server"too