feat(rsc): improve directive transform primitives

[james-elicx]

Summary

Strips this PR back to low-level RSC transform improvements that framework integrations can compose externally.

This is now based on #1310. That PR owns the plugin/server-reference registration story through server reference claims; this PR intentionally does not add or keep an in-core serverFunctionDirectives API.

What is included

• support inline directive object methods and static class methods in transformHoistInlineDirective
• opt-in stable generated names for wrapped inline directive functions
• export wrapped inline hoists when a transform needs them to be referenceable
• richer function/export metadata for transform consumers
• validation options for unsupported inline directive expressions and sync functions

Validation

• pnpm --dir packages/plugin-rsc exec vitest run src/transforms/hoist.test.ts src/transforms/wrap-export.test.ts src/transforms/server-action.test.ts
• pnpm --dir packages/plugin-rsc build
• pnpm --dir packages/plugin-rsc tsc
• pnpm format --check

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vitejs/plugin-react@1246

npm i https://pkg.pr.new/@vitejs/plugin-rsc@1246

npm i https://pkg.pr.new/@vitejs/plugin-react-swc@1246

commit: 50eaf47

[hi-ogawa]

Can you explain higher level motivation (than implementation explanation)? From a quick look, I cannot tell things like:

• whether this includes some fixes in existing transforms wrap/hoist and thus existing builtin "use server" support.
• whether new plugin vitePluginServerFunctionDirectives needs to be inside rsc plugin. for example, if we land changes in transforms/*, then can you build vitePluginServerFunctionDirectives outside based on the exported transform utils?
• whether vitePluginServerFunctionDirectives has some new mechanism compared to current builtin "use server" support that also benefit for builtin use server to have.

[james-elicx]

Can you explain higher level motivation (than implementation explanation)? From a quick look, I cannot tell things like:

• whether this includes some fixes in existing transforms wrap/hoist and thus existing builtin "use server" support.

Yeah, there's some improvements to how object & class methods and named expressions are treated, which the 'use server' directive will benefit from. The other part is how certain forms that can't be hoisted safely are handled and tracking metadata about parameters, which is mainly for custom directives to use. I can break those out into separate PRs if needed.

• whether new plugin vitePluginServerFunctionDirectives needs to be inside rsc plugin. for example, if we land changes in transforms/*, then can you build vitePluginServerFunctionDirectives outside based on the exported transform utils?

I believe we may be able to, however, the more I was looking at some of the 'use cache' directive stuff we were having to do as I started fixing bugs, the more it felt like we were touching more and more on the internals of how the plugin processes directives.

By that, I'm referring to:

• server reference manifests.
• the runtime proxies.
• the encryption/encoding behaviour for closures.
• some scope tracking.

It felt a lot to me like we began implementing our own generic approach to register custom directives rather than there being an API for one, and in the process, attaching onto everything the RSC plugin was already doing internally.

• whether vitePluginServerFunctionDirectives has some new mechanism compared to current builtin "use server" support that also benefit for builtin use server to have.

The 'use server' directive could actually be changed to route through this new plugin as well. I was originally going to do that and have one single way for people to register custom directives that was also used for registering the 'use server' directive by default. The way this works was based on how that directive works as its starting point.

Couldn't help but feel a tad risky without proving the new approach works well before doing that...

[james-elicx]

Thanks for the prereleases :)

Exposed a couple of issues when integrating which codex has worked though.

Changes pushed to allow importing an additional runtime for the wrap callback to use, and adding the (opt-in) ability to allow cached RSC values containing server references to be replayed/re-rendered by frameworks by preserving them, and also fixing some issues with references across environments.

You can see what the framework side of this might look like in https://github.com/cloudflare/vinext/pull/1871/changes

Sorry about that.

[ivogt]

I think I might have over-worried earlier — I thought that landing this pr it would change how server actions "use server" (e.g. getNormalizedId and serverReferenceMetaMap shape) get handled for us. It seems like it doesn't. Also that "use cache" (by adding the registerServerReference(...)) would effectively get baked into the plugin so we couldn't keep our own transform. But the transforms/* utils stay exported and the new API is opt-in, so our external, server-local transform should keep working.

Appreciate you walking through it.

One genuinely useful flag while you're in the shared transforms: transformWrapExport now returns meta.isFunction as boolean | undefined instead of defaulting to false. Consumers branching on meta.isFunction === false (we do, in our wrap filter) shift behavior — a call-expression-initialized export comes back undefined now, not false. Might be intended; we'll adapt it anyway.

[james-elicx]

One genuinely useful flag while you're in the shared transforms: transformWrapExport now returns meta.isFunction as boolean | undefined instead of defaulting to false. Consumers branching on meta.isFunction === false (we do, in our wrap filter) shift behavior — a call-expression-initialized export comes back undefined now, not false. Might be intended; we'll adapt it anyway.

The types for isFunction are already boolean | undefined in main, which means your consumption of the function would be making an incorrect assumption if it thinks that could never be possible, as the public API explicitly states that it could be undefined.

• false -> we know it's definitely not a function.
• true -> we know it is a function.
• undefined -> we don't know if it's a function.

https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-rsc/src/transforms/wrap-export.ts#L8

In fact, I think this could potentially be an improvement by honouring the public types. There are cases where the plugin may not be able to statically analyse the value of a variable as a function. Take factory functions that return callbacks as an example - there's a decent chance they might not be statically analysed as resulting the value of the variable being a function. I haven't tested whether they are or not, but the public types would allow you to have better control over that scenario if it's accurately returning undefined.

[ivogt]

@james-elicx indeed that was something I missed and I had to fix that in our implementation. Thank you for clarifying.