diff --git a/packages/plugin-rsc/src/transforms/hoist.test.ts b/packages/plugin-rsc/src/transforms/hoist.test.ts index 3dde6bc91..ef5b166f7 100644 --- a/packages/plugin-rsc/src/transforms/hoist.test.ts +++ b/packages/plugin-rsc/src/transforms/hoist.test.ts @@ -1,6 +1,6 @@ import path from 'node:path' import { parseAstAsync } from 'vite' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { transformHoistInlineDirective } from './hoist' import { debugSourceMap } from './test-utils' @@ -466,11 +466,11 @@ export async function kv() { }), ).toMatchInlineSnapshot(` " - export const none = /* #__PURE__ */ $$register($$hoist_0_none, "", "$$hoist_0_none", {"directiveMatch":["use cache",null]}); + export const none = /* #__PURE__ */ $$register($$hoist_0_none, "", "$$hoist_0_none", {"directiveMatch":["use cache",null],"hasBoundArgs":false,"parameters":{"count":0,"hasRest":false}}); - export const fs = /* #__PURE__ */ $$register($$hoist_1_fs, "", "$$hoist_1_fs", {"directiveMatch":["use cache: fs",": fs"]}); + export const fs = /* #__PURE__ */ $$register($$hoist_1_fs, "", "$$hoist_1_fs", {"directiveMatch":["use cache: fs",": fs"],"hasBoundArgs":false,"parameters":{"count":0,"hasRest":false}}); - export const kv = /* #__PURE__ */ $$register($$hoist_2_kv, "", "$$hoist_2_kv", {"directiveMatch":["use cache: kv",": kv"]}); + export const kv = /* #__PURE__ */ $$register($$hoist_2_kv, "", "$$hoist_2_kv", {"directiveMatch":["use cache: kv",": kv"],"hasBoundArgs":false,"parameters":{"count":0,"hasRest":false}}); ;async function $$hoist_0_none() { "use cache"; @@ -508,4 +508,64 @@ export async function test() { " `) }) + + it('uses stable names and reports bound arguments', async () => { + const input = ` +async function outer(value) { + return async function cached() { + "use cache"; + return value; + }; +} +` + const ast = await parseAstAsync(input) + const runtime = vi.fn((value: string) => value) + const first = transformHoistInlineDirective(input, ast, { + directive: 'use cache', + stableName: true, + runtime, + }) + const second = transformHoistInlineDirective(input, ast, { + directive: 'use cache', + stableName: true, + runtime: (value) => value, + }) + expect(runtime).toHaveBeenCalledWith( + expect.any(String), + expect.any(String), + expect.objectContaining({ hasBoundArgs: true }), + ) + expect(first.names).toEqual(second.names) + expect(first.names[0]).toMatch(/^\$\$hoist_[a-f0-9]{12}_0_cached$/) + }) + + it('supports object and static class methods', async () => { + const input = ` +const object = { + async ["cached"]() { "use cache"; return 1 }, +}; +class Cache { + static async ["cached"]() { "use cache"; return 2 } +} +` + const transformed = await testTransform(input, { + directive: 'use cache', + }) + expect(transformed).toContain('"cached": /* #__PURE__ */') + expect(transformed).toContain('static ["cached"] = /* #__PURE__ */') + await parseAstAsync(transformed!) + }) + + it('rejects instance, private, getter and setter methods', async () => { + for (const input of [ + `class Cache { async cached() { "use cache" } }`, + `class Cache { static async #cached() { "use cache" } }`, + `const cache = { get cached() { "use cache"; return 1 } }`, + `class Cache { static set cached(value) { "use cache" } }`, + ]) { + await expect( + testTransform(input, { directive: 'use cache' }), + ).rejects.toThrow(/not allowed/) + } + }) }) diff --git a/packages/plugin-rsc/src/transforms/hoist.ts b/packages/plugin-rsc/src/transforms/hoist.ts index 8d1665a70..c131f9d4b 100644 --- a/packages/plugin-rsc/src/transforms/hoist.ts +++ b/packages/plugin-rsc/src/transforms/hoist.ts @@ -1,6 +1,7 @@ import { tinyassert } from '@hiogawa/utils' import type { Program, + BlockStatement, Literal, Node, MemberExpression, @@ -8,7 +9,9 @@ import type { } from 'estree' import { walk } from 'estree-walker' import MagicString from 'magic-string' +import { hashString } from '../plugins/utils' import { buildScopeTree, type ScopeTree } from './scope' +import type { FunctionParameters } from './wrap-export' export function transformHoistInlineDirective( input: string, @@ -21,13 +24,20 @@ export function transformHoistInlineDirective( runtime: ( value: string, name: string, - meta: { directiveMatch: RegExpMatchArray }, + meta: { + directiveMatch: RegExpMatchArray + hasBoundArgs: boolean + parameters: FunctionParameters + }, ) => string directive: string | RegExp rejectNonAsyncFunction?: boolean encode?: (value: string) => string decode?: (value: string) => string noExport?: boolean + exportWrappedHoist?: boolean + stableName?: boolean + rejectForbiddenExpressions?: boolean }, ): { output: MagicString @@ -45,6 +55,7 @@ export function transformHoistInlineDirective( const scopeTree = buildScopeTree(ast) const names: string[] = [] + const signatureCounts = new Map() walk(ast, { enter(node, parent) { @@ -64,13 +75,64 @@ export function transformHoistInlineDirective( }, ) } + if (options.rejectForbiddenExpressions) { + validateForbiddenExpressions(node.body, match[0]) + } + + const isObjectMethod = + node.type === 'FunctionExpression' && + parent?.type === 'Property' && + (parent.method || parent.kind !== 'init') + const isClassMethod = + node.type === 'FunctionExpression' && + parent?.type === 'MethodDefinition' + if (isClassMethod && !parent.static) { + throw Object.assign( + new Error( + `It is not allowed to define inline ${JSON.stringify(match[0])} annotated class instance methods. Use a function, object method property, or static class method instead.`, + ), + { pos: parent.start }, + ) + } + if (isClassMethod && parent.key.type === 'PrivateIdentifier') { + throw Object.assign( + new Error( + `It is not allowed to define inline ${JSON.stringify(match[0])} annotated private class methods.`, + ), + { pos: parent.start }, + ) + } + if ( + (isObjectMethod && parent.kind !== 'init') || + (isClassMethod && parent.kind !== 'method') + ) { + throw Object.assign( + new Error( + `It is not allowed to define inline ${JSON.stringify(match[0])} annotated getters or setters.`, + ), + { pos: parent.start }, + ) + } const declName = node.type === 'FunctionDeclaration' && node.id.name + const expressionName = + node.type === 'FunctionExpression' ? node.id?.name : undefined + const methodName = + (isObjectMethod || isClassMethod) && + (parent.key.type === 'Identifier' || parent.key.type === 'Literal') + ? String( + parent.key.type === 'Identifier' + ? parent.key.name + : parent.key.value, + ) + : undefined const originalName = declName || + methodName || (parent?.type === 'VariableDeclarator' && parent.id.type === 'Identifier' && parent.id.name) || + expressionName || 'anonymous_server_function' const bindVars = getBindVars(node, scopeTree) @@ -92,35 +154,73 @@ export function transformHoistInlineDirective( } // append a new `FunctionDeclaration` at the end + let nameKey = String(names.length) + if (options.stableName) { + const signature = `${originalName}:${input.slice(node.start, node.end)}` + const signatureCount = signatureCounts.get(signature) ?? 0 + signatureCounts.set(signature, signatureCount + 1) + nameKey = `${hashString(signature)}_${signatureCount}` + } const newName = - `$$hoist_${names.length}` + (originalName ? `_${originalName}` : '') + `$$hoist_${nameKey}` + (originalName ? `_${originalName}` : '') names.push(newName) + const implementationName = options.exportWrappedHoist + ? `${newName}$$impl` + : newName output.update( node.start, node.body.start, - `\n;${options.noExport ? '' : 'export '}${ + `\n;${options.noExport || options.exportWrappedHoist ? '' : 'export '}${ node.async ? 'async ' : '' - }function${node.generator ? '*' : ''} ${newName}(${newParams}) `, + }function${node.generator ? '*' : ''} ${implementationName}(${newParams}) `, ) output.appendLeft( node.end, - `;\n/* #__PURE__ */ Object.defineProperty(${newName}, "name", { value: ${JSON.stringify( + `;\n/* #__PURE__ */ Object.defineProperty(${implementationName}, "name", { value: ${JSON.stringify( originalName, )} });\n`, ) output.move(node.start, node.end, input.length) // replace original declartion with action register + bind - let newCode = `/* #__PURE__ */ ${runtime(newName, newName, { - directiveMatch: match, - })}` + let wrappedCode = `/* #__PURE__ */ ${runtime( + implementationName, + newName, + { + directiveMatch: match, + hasBoundArgs: bindVars.length > 0, + parameters: { + count: node.params.length, + hasRest: node.params.some( + (parameter) => parameter.type === 'RestElement', + ), + }, + }, + )}` + let newCode = wrappedCode + if (options.exportWrappedHoist) { + output.prepend(`export const ${newName} = ${wrappedCode};\n`) + newCode = newName + } if (bindVars.length > 0) { const bindArgs = options.encode ? options.encode('[' + bindVars.map((b) => b.expr).join(', ') + ']') : bindVars.map((b) => b.expr).join(', ') newCode = `${newCode}.bind(null, ${bindArgs})` } - if (declName) { + if (isObjectMethod) { + output.update( + parent.start, + node.start, + `${input.slice(parent.key.start, parent.key.end)}: `, + ) + } else if (isClassMethod) { + const key = parent.computed + ? `[${input.slice(parent.key.start, parent.key.end)}]` + : input.slice(parent.key.start, parent.key.end) + output.update(parent.start, node.start, `static ${key} = `) + newCode += ';' + } else if (declName) { newCode = `const ${declName} = ${newCode};` if (parent?.type === 'ExportDefaultDeclaration') { output.remove(parent.start, node.start) @@ -132,10 +232,44 @@ export function transformHoistInlineDirective( }, }) - return { - output, - names, - } + return { output, names } +} + +function validateForbiddenExpressions(body: BlockStatement, directive: string) { + walk(body, { + enter(node, parent) { + if ( + node !== body && + (node.type === 'FunctionDeclaration' || + node.type === 'FunctionExpression') + ) { + this.skip() + return + } + const isJsxDevSourceThis = + node.type === 'ThisExpression' && + parent?.type === 'CallExpression' && + parent.arguments.at(-1) === node && + parent.callee.type === 'Identifier' && + /(?:^|_)jsxDEV$/.test(parent.callee.name) + const expression = + node.type === 'ThisExpression' && !isJsxDevSourceThis + ? 'this' + : node.type === 'Super' + ? 'super' + : node.type === 'Identifier' && node.name === 'arguments' + ? 'arguments' + : undefined + if (expression) { + throw Object.assign( + new Error( + `${JSON.stringify(directive)} functions cannot use ${JSON.stringify(expression)}.`, + ), + { pos: node.start }, + ) + } + }, + }) } const exactRegex = (s: string): RegExp => diff --git a/packages/plugin-rsc/src/transforms/server-action.test.ts b/packages/plugin-rsc/src/transforms/server-action.test.ts index ff6b343cb..7ad891836 100644 --- a/packages/plugin-rsc/src/transforms/server-action.test.ts +++ b/packages/plugin-rsc/src/transforms/server-action.test.ts @@ -26,7 +26,7 @@ test('normalizes inline server reference names', async () => { ) expect(result).toMatchObject({ - names: ['$$hoist_0_anonymous_server_function'], - referenceNames: ['$$hoist_0_anonymous_server_function'], + names: ['$$hoist_0_action'], + referenceNames: ['$$hoist_0_action'], }) }) diff --git a/packages/plugin-rsc/src/transforms/wrap-export.test.ts b/packages/plugin-rsc/src/transforms/wrap-export.test.ts index 1e63d1e59..9fa6e54fa 100644 --- a/packages/plugin-rsc/src/transforms/wrap-export.test.ts +++ b/packages/plugin-rsc/src/transforms/wrap-export.test.ts @@ -379,15 +379,36 @@ export default cached; test('runtime export meta', async () => { const examples: [input: string, expected: unknown[]][] = [ - [`export function Fn() {}`, [{ isFunction: true, declName: 'Fn' }]], + [ + `export function Fn() {}`, + [ + { + isFunction: true, + declName: 'Fn', + parameters: { count: 0, hasRest: false }, + }, + ], + ], [`export class Cls {}`, [{ isFunction: false, declName: 'Cls' }]], [ `export const Arrow = () => {}`, - [{ isFunction: true, declName: 'Arrow' }], + [ + { + isFunction: true, + declName: 'Arrow', + parameters: { count: 0, hasRest: false }, + }, + ], ], [ `export const FnExpression = function () {}`, - [{ isFunction: true, declName: 'FnExpression' }], + [ + { + isFunction: true, + declName: 'FnExpression', + parameters: { count: 0, hasRest: false }, + }, + ], ], [ `export const Literal = 1`, @@ -411,7 +432,11 @@ export default cached; [ `export const MultiFn = () => {}, MultiValue = 1, MultiUnknown = getValue()`, [ - { isFunction: true, declName: 'MultiFn' }, + { + isFunction: true, + declName: 'MultiFn', + parameters: { count: 0, hasRest: false }, + }, { isFunction: false, declName: 'MultiValue' }, { declName: 'MultiUnknown' }, ], @@ -422,10 +447,14 @@ export default cached; { isFunction: true, declName: 'Page', + parameters: { count: 0, hasRest: false }, }, ], ], - [`export default function () {}`, [{ isFunction: true }]], + [ + `export default function () {}`, + [{ isFunction: true, parameters: { count: 0, hasRest: false } }], + ], [ `export default class Page {}`, [ @@ -441,6 +470,7 @@ export default cached; [ { isFunction: true, + parameters: { count: 0, hasRest: false }, }, ], ], @@ -457,10 +487,14 @@ export default cached; [ { defaultExportIdentifierName: 'Page', + parameters: { count: 0, hasRest: false }, }, ], ], - [`const id = async () => {}; export { id }`, [{}]], + [ + `const id = async () => {}; export { id }`, + [{ isFunction: true, parameters: { count: 0, hasRest: false } }], + ], [`export { id } from './dep'`, [{}]], ] diff --git a/packages/plugin-rsc/src/transforms/wrap-export.ts b/packages/plugin-rsc/src/transforms/wrap-export.ts index 4e7bd6c86..b0a950220 100644 --- a/packages/plugin-rsc/src/transforms/wrap-export.ts +++ b/packages/plugin-rsc/src/transforms/wrap-export.ts @@ -3,7 +3,12 @@ import type { ExportDefaultDeclaration, Node, Program } from 'estree' import MagicString from 'magic-string' import { extractNames, validateNonAsyncFunction } from './utils' -type ExportMeta = { +export type FunctionParameters = { + count: number + hasRest: boolean +} + +export type ExportMeta = { /** * The local declaration name when statically available. * @@ -32,6 +37,7 @@ type ExportMeta = { * - `undefined` for `export default () => {}` */ defaultExportIdentifierName?: string + parameters?: FunctionParameters } type ExportWithMeta = { name: string; meta: ExportMeta } @@ -60,6 +66,26 @@ export function transformWrapExport( const exportNames: string[] = [] const toAppend: string[] = [] const filter = options.filter ?? (() => true) + const localFunctionParameters = new Map() + + for (const node of ast.body) { + if (node.type === 'FunctionDeclaration' && node.id) { + localFunctionParameters.set(node.id.name, getFunctionParameters(node)) + } else if (node.type === 'VariableDeclaration') { + for (const declaration of node.declarations) { + if ( + declaration.id.type === 'Identifier' && + (declaration.init?.type === 'ArrowFunctionExpression' || + declaration.init?.type === 'FunctionExpression') + ) { + localFunctionParameters.set( + declaration.id.name, + getFunctionParameters(declaration.init), + ) + } + } + } + } function wrapSimple(start: number, end: number, exports: ExportWithMeta[]) { const filteredExports = exports.map((item) => ({ @@ -129,6 +155,10 @@ export function transformWrapExport( const meta: ExportMeta = { isFunction: getIsFunction(node.declaration), declName: name, + parameters: + node.declaration.type === 'FunctionDeclaration' + ? getFunctionParameters(node.declaration) + : undefined, } if (filter(name, meta)) { validateNonAsyncFunction(options, node.declaration) @@ -151,11 +181,17 @@ export function transformWrapExport( decl.id.type === 'Identifier' && decl.init ? getIsFunction(decl.init) : undefined + const parameters = + decl.id.type === 'Identifier' && + (decl.init?.type === 'ArrowFunctionExpression' || + decl.init?.type === 'FunctionExpression') + ? getFunctionParameters(decl.init) + : undefined const declarationExports: ExportWithMeta[] = extractNames( decl.id, ).map((name) => ({ name, - meta: { isFunction, declName: name }, + meta: { isFunction, declName: name, parameters }, })) exports.push(...declarationExports) if ( @@ -202,7 +238,12 @@ export function transformWrapExport( { pos: spec.exported.start }, ) } - wrapExport(spec.local.name, spec.exported.name) + wrapExport(spec.local.name, spec.exported.name, { + isFunction: localFunctionParameters.has(spec.local.name) + ? true + : undefined, + parameters: localFunctionParameters.get(spec.local.name), + }) } } } @@ -233,6 +274,7 @@ export function transformWrapExport( let isFunction: boolean | undefined let declName: string | undefined let defaultExportIdentifierName: string | undefined + let parameters: FunctionParameters | undefined if ( (node.declaration.type === 'FunctionDeclaration' || node.declaration.type === 'ClassDeclaration') && @@ -243,20 +285,32 @@ export function transformWrapExport( output.remove(node.start, node.declaration.start) isFunction = getIsFunction(node.declaration) declName = node.declaration.id.name + parameters = + node.declaration.type === 'FunctionDeclaration' + ? getFunctionParameters(node.declaration) + : undefined } else { // otherwise we can introduce new variable localName = '$$default' output.update(node.start, node.declaration.start, 'const $$default = ') if (node.declaration.type === 'Identifier') { defaultExportIdentifierName = node.declaration.name + parameters = localFunctionParameters.get(node.declaration.name) } else { isFunction = getIsFunction(node.declaration) + parameters = + node.declaration.type === 'FunctionDeclaration' || + node.declaration.type === 'FunctionExpression' || + node.declaration.type === 'ArrowFunctionExpression' + ? getFunctionParameters(node.declaration) + : undefined } } const defaultMeta: ExportMeta = { isFunction, declName, defaultExportIdentifierName, + parameters, } if (filter('default', defaultMeta)) { validateNonAsyncFunction(options, node.declaration) @@ -292,3 +346,12 @@ function getIsFunction( return false } } + +function getFunctionParameters(node: { + params: import('estree').Pattern[] +}): FunctionParameters { + return { + count: node.params.length, + hasRest: node.params.some((parameter) => parameter.type === 'RestElement'), + } +}