diff --git a/.oxfmtrc.json b/.oxfmtrc.json index 9fa524588..ec2b1779f 100644 --- a/.oxfmtrc.json +++ b/.oxfmtrc.json @@ -9,7 +9,7 @@ "groups": [["builtin"], ["external"]] }, "ignorePatterns": [ - "*.snap.json", + "*.snap.*", "typescript-eslint/", "packages/*/CHANGELOG.md", "playground-temp/", diff --git a/packages/plugin-rsc/docs/notes/2026-04-05-member-chain-optional-computed-follow-up.md b/packages/plugin-rsc/docs/notes/2026-04-05-member-chain-optional-computed-follow-up.md new file mode 100644 index 000000000..15bb13368 --- /dev/null +++ b/packages/plugin-rsc/docs/notes/2026-04-05-member-chain-optional-computed-follow-up.md @@ -0,0 +1,363 @@ +# Member-Chain Follow-Up: Optional and Computed Access + +## Goal + +Track the next step after plain non-computed member-chain binding: + +- optional chaining, e.g. `x?.y.z`, `x.y?.z` +- computed access, e.g. `x[y].z`, `x.y[k]` + +This note is intentionally a follow-up to the plain member-chain work in +[2026-04-05-rsc-member-chain-binding-plan.md](./2026-04-05-rsc-member-chain-binding-plan.md). +It is not part of the current cleanup / plain-chain implementation. + +## Current state + +The current implementation is intentionally narrow: + +- plain non-computed member chains like `x.y.z` are captured precisely +- unsupported hops stop capture at the last safe prefix +- examples: + - `x?.y.z` -> bind `x` + - `a.b?.c` -> bind `{ b: a.b }` + - `x[y].z` -> bind `x` + +This is a reasonable conservative failure mode, but it is not full support. + +## Why this needs a separate design + +The current `BindPath` shape in [src/transforms/hoist.ts](../../src/transforms/hoist.ts) +is effectively: + +```ts +type BindPath = { + key: string + segments: string[] +} +``` + +That is enough for `x.y.z` because codegen can reconstruct the bind expression +from the root identifier plus dot segments. + +It is not enough for: + +- `x?.y.z` +- `x.y?.z` +- `x[y].z` +- `x?.[y]` + +The missing information is not cosmetic. It changes semantics. + +### Optional chaining + +Each hop needs to preserve whether access is optional. + +Example: + +```js +x?.y.z +``` + +Reconstructing this as `x.y.z` is wrong because the bind-time access becomes +stricter than the original expression. + +### Computed access + +Each computed hop needs the property expression, not just a string segment. + +Example: + +```js +x[y].z +``` + +There is no way to reconstruct this faithfully from `["y", "z"]`, because the +first `y` is an expression, not a property name. + +### Computed key expressions also have their own closure semantics + +Computed access is not only a codegen problem. The key expression itself may +close over outer variables, or it may be local to the action. + +Outer-scope key: + +```js +function outer() { + let key = 'x' + let obj = {} + async function action() { + 'use server' + return obj[key] + } +} +``` + +Both `obj` and `key` are outer captures. + +Action-local key: + +```js +function outer() { + let obj = {} + async function action() { + 'use server' + let key = 'x' + return obj[key] + } +} +``` + +Only `obj` is an outer capture; `key` is local to the action. + +So any future `obj[expr]` support must treat the computed key as an ordinary +expression with its own scope resolution, not just as a printable suffix on a +member path. + +## Minimum data model change + +To support these cases, `BindPath` needs richer per-hop metadata. + +Sketch: + +```ts +type BindSegment = + | { kind: 'property'; name: string; optional: boolean } + | { kind: 'computed'; expr: Node; optional: boolean } + +type BindPath = { + key: string + segments: BindSegment[] +} +``` + +This is enough to represent: + +- `.foo` +- `?.foo` +- `[expr]` +- `?.[expr]` + +The exact `key` design is still open. It only needs to support dedupe among +captures that are semantically comparable. + +## Required implementation areas + +### 1. `scope.ts`: capture shape + +In [src/transforms/scope.ts](../../src/transforms/scope.ts), +`getOutermostBindableReference()` currently accumulates only plain +non-computed member chains and stops at unsupported hops. + +To support optional/computed access, capture analysis must preserve richer +member-hop metadata instead of reducing everything to `Identifier` or +`MemberExpression` with plain identifier-name segments. + +That likely means changing either: + +- what `referenceToNode` stores, or +- adding a new structured capture representation derived from the AST + +### 2. `hoist.ts`: path extraction + +In [src/transforms/hoist.ts](../../src/transforms/hoist.ts), +`memberExpressionToPath()` currently extracts only `string[]` segments. + +That helper would need to become a structured extractor that records: + +- property vs computed +- optional vs non-optional +- enough information to regenerate the bind expression + +### 3. Dedupe semantics + +Current prefix dedupe is straightforward for plain dot paths: + +- `x.y` covers `x.y.z` +- `x` covers everything below it + +With optional/computed access, dedupe needs clearer rules. + +Questions: + +- does `x.y` cover `x.y?.z`? +- does `x[y]` cover `x[y].z` only when the computed key expression is identical? +- how should keys be normalized for comparison? + +The current antichain logic should not be reused blindly. + +### 3a. Support boundary for `obj[expr]` + +This is still intentionally unresolved. + +Possible support levels: + +1. Keep current safe-prefix bailout only. + Examples: + - `obj[key]` -> bind `obj`, bind `key` separately if it is an outer capture + - `obj[key].value` -> bind `obj`, bind `key` separately if needed + +2. Support exact computed member captures only for simple shapes. + Examples: + - `obj[key]` + - `obj[key].value` + but only when we have a clear representation for both the base object and the + key expression. + +3. Support computed access as a first-class bind path. + This would require fully defining: + - path equality + - prefix coverage + - codegen for bind expressions + - partial-object synthesis, if still applicable + +At the moment, the note does not assume we will reach (3). It is entirely +reasonable to stop at (1) or (2) if the semantics and implementation cost of +full computed-path support are not compelling. + +### 4. Bind-expression codegen + +Current codegen only needs: + +- `root` +- `segments: string[]` + +and synthesizes: + +```ts +root + segments.map((segment) => `.${segment}`).join('') +``` + +That must be replaced with codegen that can emit: + +- `.foo` +- `?.foo` +- `[expr]` +- `?.[expr]` + +### 5. Partial-object synthesis + +This is the hardest part. + +For plain member paths, partial-object synthesis is natural: + +```js +{ + y: { + z: x.y.z + } +} +``` + +For computed access, synthesis is less obvious: + +```js +x[k].z +``` + +Questions: + +- should this become an object with computed keys? +- should computed paths fall back to broader binding even after we support + recognizing them? +- does partial-object binding remain the right representation for these cases? + +This is where the design may need to diverge from plain member chains. + +### 6. Comparison with Next.js + +Relevant prior art is documented in +[scope-manager-research/nextjs.md](./scope-manager-research/nextjs.md). + +Important comparison points: + +- Next.js already models optional member access in its `NamePart` structure. +- Next.js does not support computed properties in the captured member-path + model. +- Next.js member-path capture is deliberately limited to member chains like + `foo.bar.baz`. + +That means: + +- optional chaining has direct prior art in Next.js's capture model +- computed access does not; if we support it, we are going beyond the current + Next.js design + +This should affect scoping decisions for the follow-up: + +- optional support is an extension of an already-established member-path model +- computed support is a materially larger design question, especially once key + expression scope and dedupe semantics are included + +## Safe intermediate target + +If we want a minimal correctness-first follow-up: + +1. keep the current safe-prefix bailout behavior +2. add explicit tests for optional/computed cases +3. only implement richer capture metadata once codegen and dedupe rules are + agreed + +That avoids regressing semantics while leaving room for a more precise design. + +## Temporary conclusion + +Current working direction: + +- likely support optional chaining next, to align with Next.js's existing + member-path behavior +- keep computed access as a separate, open design problem for now + +Rationale: + +- optional chaining already has prior art in Next.js's capture model +- computed access is materially more complex because it mixes: + - key-expression scope resolution + - path equality / dedupe rules + - bind-expression codegen + - unclear partial-object synthesis semantics + +So the likely near-term path is: + +1. support optional member chains +2. keep current conservative behavior for computed access +3. revisit computed support only if there is a clear use case and a concrete + design that handles key-expression closure semantics correctly + +## Suggested first questions before coding + +1. Optional chains: + Should the first supported version preserve optional syntax exactly in the + bound expression, or should optional hops continue to bail out? + +2. Computed access: + Do we want exact support for `x[y].z`, or only a less coarse bailout than + binding the whole root? + +3. Binding shape: + Is partial-object synthesis still the preferred strategy for computed access, + or does this push us toward a different representation? + +4. Computed key scope: + If we support `obj[expr]`, what is the intended contract for the key + expression? + Specifically: + - must outer variables used in `expr` always be captured independently? + - do we need a representation that distinguishes outer `key` from + action-local `key` when deciding support and dedupe? + +5. Comparison target: + Do we want to stay aligned with Next.js and continue treating computed access + as out of scope, or intentionally support a broader feature set? + +## Candidate tests + +Add focused hoist fixtures for: + +1. `x?.y.z` +2. `x.y?.z` +3. `x?.y?.z` +4. `x[y].z` +5. `x.y[k]` +6. `x[y]?.z` +7. `a.b?.c` as a safe-prefix bailout baseline +8. `a[b].c` as a safe-prefix bailout baseline diff --git a/packages/plugin-rsc/docs/notes/2026-04-05-rsc-member-chain-binding-plan.md b/packages/plugin-rsc/docs/notes/2026-04-05-rsc-member-chain-binding-plan.md new file mode 100644 index 000000000..77fcd0066 --- /dev/null +++ b/packages/plugin-rsc/docs/notes/2026-04-05-rsc-member-chain-binding-plan.md @@ -0,0 +1,441 @@ +# RSC Member-Chain Binding Plan + +## Goal + +Implement the feature gap called out in [src/transforms/scope.ts#L129](../../src/transforms/scope.ts#L129) and the Next.js comparison note at [scope-manager-research/nextjs.md#L126](./scope-manager-research/nextjs.md#L126): + +- track `config.api.key` as a closure capture instead of only `config` +- keep declaration resolution anchored on the root identifier (`config`) +- preserve current shadowing correctness from the custom scope tree + +## RSC binding semantics + +Understanding why binding matters requires knowing what happens at runtime. + +When a server component renders a `'use server'` function, the transform produces: + +```js +const action = $$register($$hoist_0_action, ...).bind(null, config.api.key) +``` + +The `.bind(null, ...)` arguments are evaluated **at render time on the server**. The resulting bound function reference is then serialized into the RSC payload and sent to the client. The client holds an opaque reference. When the user invokes the action (e.g. submits a form), the client sends the action ID and the bound args back to the server, which deserializes them and reconstructs the call. + +So bound values travel over the network: **they must be React-serializable** (plain objects, arrays, primitives, Dates, Sets, Maps — but not class instances with methods, functions, or other non-serializable types). + +The `encode`/`decode` option in `transformHoistInlineDirective` is a separate concern — it is an encryption layer applied on top of this transport, not the transport itself. + +### Implication for member-chain binding + +Binding the root object (`config`) means the whole object travels the wire. If `config` contains non-serializable parts, the action will fail at runtime. + +Binding only the needed paths is therefore preferable: the bound value is more likely to be a primitive or a small serializable subtree. + +Both the partial-object approach and the synthetic-local approach are safer than the current behavior of binding the full root object. The difference between them is shape, not serializability. + +## Key finding + +This is **not** a `scope.ts`-only change. + +Current `getBindVars` in [src/transforms/hoist.ts#L171](../../src/transforms/hoist.ts#L171) returns plain strings that are used in two different roles: + +1. as the bound expression list for `.bind(null, ...)` +2. as the hoisted function parameter names / decode destructuring targets + +That works for identifiers like `value`, but breaks for member paths like `config.api` because: + +- `config.api` is valid as a bound expression +- `function hoisted(config.api) {}` is invalid syntax +- `const [config.api] = decode(...)` is also invalid syntax + +The chosen design below solves this by keeping the root identifier name as the parameter and synthesizing the bind expression as a partial object. + +## Current code points + +- Scope collection TODO: [src/transforms/scope.ts#L129](../../src/transforms/scope.ts#L129) +- Scope tree types: [src/transforms/scope.ts#L37](../../src/transforms/scope.ts#L37) +- Bind-var extraction: [src/transforms/hoist.ts#L171](../../src/transforms/hoist.ts#L171) +- Hoist codegen uses `bindVars` as both params and bound args: [src/transforms/hoist.ts#L69](../../src/transforms/hoist.ts#L69) +- Scope serializer assumes every reference is an `Identifier`: [src/transforms/scope.test.ts#L62](../../src/transforms/scope.test.ts#L62) + +## Chosen design: partial-object binding under the root name + +Keep the original root variable name as the hoisted function parameter. Instead of binding the whole root object, synthesize a partial object at the call site that reconstructs just enough shape for the accessed paths. + +Example: + +```js +function Page() { + const config = getConfig() + + async function action() { + 'use server' + return config.api.key + } +} +``` + +Output: + +```js +function Page() { + const config = getConfig() + const action = $$register($$hoist_0_action, '', '$$hoist_0_action').bind( + null, + { api: { key: config.api.key } }, + ) +} + +export async function $$hoist_0_action(config) { + 'use server' + return config.api.key +} +``` + +When the root itself is accessed directly, bind it as-is (same as current behavior): + +```js +async function action() { + 'use server' + return config === globalConfig +} +// → .bind(null, config) +// → function $$hoist_0_action(config) +``` + +No body rewrite is needed. The hoisted function keeps the original source expressions. + +### Multiple paths from the same root + +When the body accesses multiple paths from the same root, the partial objects are merged: + +```js +async function action() { + 'use server' + return [config.api.key, config.user.name] +} +``` + +Output: + +```js +.bind(null, { api: { key: config.api.key }, user: { name: config.user.name } }) +export async function $$hoist_0_action(config) { + return [config.api.key, config.user.name] +} +``` + +### Dedupe: root access covers all member paths + +If the body accesses both the root and a member path from the same root, the root wins and no partial object is needed: + +```js +return [config.api.key, Object.keys(config)] +// config is accessed directly → bind config, not a partial object +``` + +This matches the prefix-dedupe rule: a shorter prefix covers all longer paths. + +## Implementation plan + +### 1. Extend `scope.ts` reference collection + +Keep `scopeToReferences` as `Map` and `referenceToDeclaredScope` as `Map` — both unchanged. Add one new field to `ScopeTree`: + +```ts +referenceToNode: Map +``` + +This maps each root reference identifier to the outermost bindable node: the identifier itself if there is no non-computed member chain, or the outermost `MemberExpression` rooted at it otherwise. + +During the walk, for each qualifying identifier, compute the outermost bindable node and store the pair: + +- `scopeToReferences` and `referenceToDeclaredScope` receive the root `Identifier` as before +- `referenceToNode` additionally stores `id → node` + +For `x.y.z`, `referenceToNode` maps `x → MemberExpression(x.y.z)`. + +For `x[y].z`, the chain is broken by the computed access, so `referenceToNode` maps `x → Identifier(x)`. + +Suggested helper in `scope.ts`: + +```ts +function getOutermostBindableReference( + id: Identifier, + parentStack: Node[], +): Identifier | MemberExpression +``` + +It can reuse the existing ancestor stack already built in `buildScopeTree`. + +### 2. Keep declaration resolution keyed by the root identifier + +`referenceToDeclaredScope` remains `Map`. + +The declaration walk stays exactly rooted on `id.name`, not the member path. This keeps current shadowing behavior intact and avoids inventing fake declarations for properties. + +### 3. Add bind-var extraction that understands member paths + +Replace the current `getBindVars(): string[]` with something richer: + +```ts +type BindVar = { + root: string // param name and dedupe key, e.g. "config" + expr: string // bind expression, e.g. "{ api: { key: config.api.key } }" or "config" +} +``` + +Filtering logic is the same as today — `scopeToReferences` and `referenceToDeclaredScope` are unchanged: + +- take the function scope's propagated references (`Identifier[]`) +- keep only those declared in ancestor scopes excluding module scope + +Then for each kept identifier, look up `referenceToNode` to get the bindable node (`Identifier` or `MemberExpression`), and extract its path for dedupe and synthesis. + +Then group by root and apply prefix dedupe to produce an antichain: + +**Antichain invariant**: no retained capture should be a prefix of another. For any two captured paths where one is a prefix of the other, discard the longer one. + +Examples: + +- `config` + `config.api.key` → keep `config` +- `config.api` + `config.api.key` → keep `config.api` +- `config.api` + `config.user.name` → keep both (neither is a prefix of the other) + +This must apply at every depth, not just at root level. Once the antichain is computed: + +- if a retained capture is the root identifier itself, bind it directly: `{ root: "config", expr: "config" }` +- otherwise, synthesize a partial object from the retained paths (see step 4) + +### 4. Synthesize partial-object bind expressions + +For each root with only member-path references (no direct root access), build a nested object literal from the antichain-deduplicated path set. + +Because the paths form an antichain, no path is a prefix of another. The trie therefore has no leaf/branch collisions: every node is either an internal node (has children only) or a leaf (a retained capture). Serialization is unambiguous. + +Example path set for `config` after dedupe: `[["api", "key"], ["user", "name"]]` + +Algorithm: + +1. Build a path tree (trie over path segments) +2. Serialize the trie to an object literal string, with each leaf node being the original source expression + +``` +["api", "key"] → api: { key: config.api.key } +["user", "name"] → user: { name: config.user.name } +merged → { api: { key: config.api.key }, user: { name: config.user.name } } +``` + +If a retained path ends at an intermediate node (e.g. `["api"]` retained, meaning `config.api` is captured directly), the value at that node is the source expression for that path, not a further-nested object: + +``` +["api"] → api: config.api +``` + +This is also what keeps the approach semantics-preserving for broader intermediate reads. For `return [config.api.key, Object.keys(config.api)]`, the reference set contains both `config.api` and `config.api.key`. Antichain dedupe keeps `config.api` and discards `config.api.key`. The bound object is `{ api: config.api }`, so the hoisted body receives the real `config.api` object and `Object.keys(config.api)` behaves correctly. + +### 5. Update hoist codegen + +After the above, hoist generation uses: + +- bound arg: the synthesized `expr` string +- param / decode local: the `root` name + +Without `decode`: + +```js +const action = $$register($$hoist_0_action, ...).bind(null, { api: { key: config.api.key } }) +export async function $$hoist_0_action(config) { + "use server" + return config.api.key +} +``` + +With `decode`: + +```js +const action = $$register(...).bind(null, encode([{ api: { key: config.api.key } }])) +export async function $$hoist_0_action($$hoist_encoded) { + "use server" + const [config] = decode($$hoist_encoded) + return config.api.key +} +``` + +### 6. Update tests + +#### `scope.test.ts` + +Serializer must support `MemberExpression` references instead of assuming `.name`. + +Suggested display format: + +- identifier: `value` +- member chain: `config.api.key` + +This will require updating snapshots under `packages/plugin-rsc/src/transforms/fixtures/scope/**`. + +#### `hoist.test.ts` + +Add focused cases for: + +1. plain member chain capture + +```js +function outer() { + const config = { api: { key: 'x' } } + async function action() { + 'use server' + return config.api.key + } +} +``` + +Expected: bind `{ api: { key: config.api.key } }`, param `config`, body unchanged. + +2. multiple paths merged + +```js +return [config.api.key, config.user.name] +``` + +Expected: bind `{ api: { key: config.api.key }, user: { name: config.user.name } }`. + +3. root access covers member path (dedupe) + +```js +return [config, config.api.key] +``` + +Expected: bind `config` only. + +4. computed boundary + +```js +return config[key].value +``` + +Expected: fall back to binding `config` (identifier-level capture). + +## Suggested scope for first implementation + +Keep the first pass intentionally narrow: + +- support only non-computed `MemberExpression` chains +- no optional chaining yet +- no broader refactor of the scope walker + +Callee trimming is required, not optional. Without it, `config.api.get()` produces `{ api: { get: config.api.get } }`, which detaches the method from its receiver and breaks `this` semantics. The capture-selection step must trim the final segment in callee position before dedupe and synthesis run. + +## Open questions before coding + +1. Callee trimming rule + +Callee trimming is required for correctness (see "Suggested scope" above). The implementation should follow Next.js: trim the final segment from any member-path capture that appears in callee position, capturing the receiver instead of the method. + +Next.js detail: + +- `visit_mut_callee` sets `in_callee = true` +- `visit_mut_expr` collects a `Name` +- when `in_callee` is true, it does `name.1.pop()` + +Additional nuance: `Name::try_from` supports `Expr::Member` and member-shaped `Expr::OptChain` but rejects `OptChainBase::Call`, so optional-call shapes are not handled by the member-path capture path. + +Relevant source: + +- [server_actions.rs#L1708](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L1708) +- [server_actions.rs#L1719](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L1719) +- [server_actions.rs#L3698](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L3698) +- [server_actions.rs#L3729](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L3729) + +This rule applies equally to both the partial-object and synthetic-local approaches — it is a capture-selection decision made before binding shape is determined. + +2. Should `scope.ts` support optional chaining now or later? + +Next.js models optional access in `NamePart`. Our TODO and current AST utilities do not. + +## Proposed execution order + +1. Add `ScopeReference` and update `scope.test.ts` serializer/snapshots. +2. Add `getBindVars` replacement that returns structured bind vars with path-tree synthesis and prefix dedupe. +3. Update hoist codegen to use `root` as param and `expr` as bind arg. +4. Add hoist tests for plain member access, multiple paths, dedupe, and computed fallback. +5. Decide whether to include callee trimming in the same change or a follow-up. + +--- + +## Appendix: synthetic-local approach (Next.js style) + +An alternative design closer to Next.js binds the exact captured leaf value and rewrites the hoisted function body to use synthetic locals instead of the original expressions. + +Example output for `return config.api.key`: + +```js +.bind(null, config.api.key) +export async function $$hoist_0_action($$hoist_arg_0) { + "use server" + return $$hoist_arg_0 +} +``` + +For multiple paths: + +```js +.bind(null, config.api.key, config.user.name) +export async function $$hoist_0_action($$hoist_arg_0, $$hoist_arg_1) { + return [$$hoist_arg_0, $$hoist_arg_1] +} +``` + +### Why more involved + +This approach requires a body rewrite pass before moving the function: every occurrence of a captured expression in the function body must be replaced with the corresponding synthetic local. This includes special handling for object shorthand: + +```js +return { config } +// config is captured and renamed to $$hoist_arg_0 +// must become: return { config: $$hoist_arg_0 } +// naive replacement would produce: return { $$hoist_arg_0 } ← wrong property name +``` + +The rewrite is also not a simple node-to-node substitution — it must operate against the final dedupe-resolved capture set, because a broader capture (e.g. `config`) subsumes narrower ones (e.g. `config.api.key`) and the body references to the narrower path must be rewritten through the broader local: + +```js +// dedupe keeps config → $$hoist_arg_0, drops config.api.key +return { config, key: config.api.key } +// becomes: +return { config: $$hoist_arg_0, key: $$hoist_arg_0.api.key } +``` + +Next.js implements this as `ClosureReplacer`: + +- [server_actions.rs#L3637](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L3637) +- [server_actions.rs#L3649](https://github.com/vercel/next.js/blob/main/crates/next-custom-transforms/src/transforms/server_actions.rs#L3649) + +### `BindVar` shape for this approach + +```ts +type BindVar = { + key: string // stable dedupe key, e.g. "config", "config.api", "config.api.key" + expr: string // source expression to bind, e.g. "config.api.key" + local: string // synthetic local used inside hoisted fn, e.g. "$$hoist_arg_0" + root: Identifier // declaration lookup still uses the root identifier +} +``` + +The separation: + +- `root` answers "which scope declared this capture?" +- `expr` answers "what value should be bound at the call site?" +- `local` answers "what identifier should replace references inside the hoisted function?" + +### Range-based rewrite variant (third approach) + +There is a middle-ground between full synthetic-local and partial-object, seen in an open PR ([vitejs/vite-plugin-react#1157](https://github.com/vitejs/vite-plugin-react/pull/1157)): + +Instead of a separate body rewrite pass, source ranges for each captured node are collected **during the analysis walk** alongside the scope information. The rewrite is then just a series of `output.update(start, end, param + suffix)` calls — no second AST traversal. + +The suffix handles prefix dedupe: if `config.cookies` is retained over `config.cookies.names`, the `config.cookies.names` occurrence is rewritten to `$$bind_0_config_cookies` + `.names`. The suffix carries the remaining path. + +Importantly, plain identifier captures (`config` with no member path) keep the original name as the param — no synthetic local, no shorthand problem. Only member-path captures get a synthetic param like `$$bind_0_config_api_key`. + +This gives lighter body rewrite than full synthetic-local (no second walk, no shorthand special-casing for member paths) while still binding leaf values rather than constructing partial objects. Worth considering if the partial-object trie synthesis turns out to be complex in practice. diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js new file mode 100644 index 000000000..1a1570227 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return x.y.fn() + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js.snap.js new file mode 100644 index 000000000..27dc77986 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-callee.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, { y: x.y }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return x.y.fn() + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js new file mode 100644 index 000000000..2d1291512 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js @@ -0,0 +1,8 @@ +function outer() { + const x = {} + const k = 'y' + async function action() { + 'use server' + return x[k].z + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js.snap.js new file mode 100644 index 000000000..9afc54644 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-computed.js.snap.js @@ -0,0 +1,11 @@ +function outer() { + const x = {} + const k = 'y' + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, x, k); +} + +;export async function $$hoist_0_action(x, k) { + 'use server' + return x[k].z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js new file mode 100644 index 000000000..09cc07821 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return [x.y.z, x.y, x.w, x.w.v] + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js.snap.js new file mode 100644 index 000000000..a249e8144 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-dedupe.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, { y: x.y, w: x.w }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return [x.y.z, x.y, x.w, x.w.v] + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js new file mode 100644 index 000000000..559631023 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js @@ -0,0 +1,15 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return x?.y.z + } +} + +function outer2() { + const a = {} + async function action() { + 'use server' + return a.b?.c + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js.snap.js new file mode 100644 index 000000000..32f221559 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-optional.js.snap.js @@ -0,0 +1,21 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, x); +} + +function outer2() { + const a = {} + const action = /* #__PURE__ */ $$register($$hoist_1_action, "", "$$hoist_1_action").bind(null, { b: a.b }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return x?.y.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); + +;export async function $$hoist_1_action(a) { + 'use server' + return a.b?.c + }; +/* #__PURE__ */ Object.defineProperty($$hoist_1_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js new file mode 100644 index 000000000..dcd5aa2d2 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return [x.__proto__.y, x.a.__proto__.b] + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js.snap.js new file mode 100644 index 000000000..8a76843dc --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-proto.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, { ["__proto__"]: { y: x.__proto__.y }, a: { ["__proto__"]: { b: x.a.__proto__.b } } }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return [x.__proto__.y, x.a.__proto__.b] + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js new file mode 100644 index 000000000..4e8bfc76b --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return [x.y.z, Object.keys(x)] + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js.snap.js new file mode 100644 index 000000000..3afddbabc --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-root.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, x); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return [x.y.z, Object.keys(x)] + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js new file mode 100644 index 000000000..d1f34fe40 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return [x.y.z, x.y.w] + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js.snap.js new file mode 100644 index 000000000..9ca0f6e20 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-siblings.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, { y: { z: x.y.z, w: x.y.w } }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return [x.y.z, x.y.w] + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js new file mode 100644 index 000000000..87ca92015 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js @@ -0,0 +1,67 @@ +function outer1() { + const x = {} + async function action() { + 'use server' + return x?.y.z + } +} + +function outer2() { + const x = {} + async function action() { + 'use server' + return x.y?.z + } +} + +function outer3() { + const x = {} + async function action() { + 'use server' + return x?.y?.z + } +} + +function outer4() { + const x = {} + const y = 'y' + async function action() { + 'use server' + return x[y].z + } +} + +function outer5() { + const x = {} + const k = 'k' + async function action() { + 'use server' + return x.y[k] + } +} + +function outer6() { + const x = {} + const y = 'y' + async function action() { + 'use server' + return x[y]?.z + } +} + +function outer7() { + const a = {} + async function action() { + 'use server' + return a.b?.c + } +} + +function outer8() { + const a = {} + const b = 'b' + async function action() { + 'use server' + return a[b].c + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js.snap.js new file mode 100644 index 000000000..24ea16e21 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain-unsupported.js.snap.js @@ -0,0 +1,91 @@ +function outer1() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, x); +} + +function outer2() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_1_action, "", "$$hoist_1_action").bind(null, { y: x.y }); +} + +function outer3() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_2_action, "", "$$hoist_2_action").bind(null, x); +} + +function outer4() { + const x = {} + const y = 'y' + const action = /* #__PURE__ */ $$register($$hoist_3_action, "", "$$hoist_3_action").bind(null, x, y); +} + +function outer5() { + const x = {} + const k = 'k' + const action = /* #__PURE__ */ $$register($$hoist_4_action, "", "$$hoist_4_action").bind(null, { y: x.y }, k); +} + +function outer6() { + const x = {} + const y = 'y' + const action = /* #__PURE__ */ $$register($$hoist_5_action, "", "$$hoist_5_action").bind(null, x, y); +} + +function outer7() { + const a = {} + const action = /* #__PURE__ */ $$register($$hoist_6_action, "", "$$hoist_6_action").bind(null, { b: a.b }); +} + +function outer8() { + const a = {} + const b = 'b' + const action = /* #__PURE__ */ $$register($$hoist_7_action, "", "$$hoist_7_action").bind(null, a, b); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return x?.y.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); + +;export async function $$hoist_1_action(x) { + 'use server' + return x.y?.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_1_action, "name", { value: "action" }); + +;export async function $$hoist_2_action(x) { + 'use server' + return x?.y?.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_2_action, "name", { value: "action" }); + +;export async function $$hoist_3_action(x, y) { + 'use server' + return x[y].z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_3_action, "name", { value: "action" }); + +;export async function $$hoist_4_action(x, k) { + 'use server' + return x.y[k] + }; +/* #__PURE__ */ Object.defineProperty($$hoist_4_action, "name", { value: "action" }); + +;export async function $$hoist_5_action(x, y) { + 'use server' + return x[y]?.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_5_action, "name", { value: "action" }); + +;export async function $$hoist_6_action(a) { + 'use server' + return a.b?.c + }; +/* #__PURE__ */ Object.defineProperty($$hoist_6_action, "name", { value: "action" }); + +;export async function $$hoist_7_action(a, b) { + 'use server' + return a[b].c + }; +/* #__PURE__ */ Object.defineProperty($$hoist_7_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js new file mode 100644 index 000000000..0c62058e5 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js @@ -0,0 +1,7 @@ +function outer() { + const x = {} + async function action() { + 'use server' + return x.y.z + } +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js.snap.js b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js.snap.js new file mode 100644 index 000000000..fb80686b6 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/hoist/member-chain.js.snap.js @@ -0,0 +1,10 @@ +function outer() { + const x = {} + const action = /* #__PURE__ */ $$register($$hoist_0_action, "", "$$hoist_0_action").bind(null, { y: { z: x.y.z } }); +} + +;export async function $$hoist_0_action(x) { + 'use server' + return x.y.z + }; +/* #__PURE__ */ Object.defineProperty($$hoist_0_action, "name", { value: "action" }); diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js new file mode 100644 index 000000000..b0be50c75 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js @@ -0,0 +1,8 @@ +let x = {} +x.y.fn() + +let a = {} +a.fn() + +let fn = {} +fn() diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js.snap.json b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js.snap.json new file mode 100644 index 000000000..9bc4745a8 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-callee.js.snap.json @@ -0,0 +1,26 @@ +{ + "type": "Program", + "declarations": [ + "a", + "fn", + "x" + ], + "references": [ + { + "name": "x", + "declaredAt": "Program", + "referenceNode": "x.y" + }, + { + "name": "a", + "declaredAt": "Program", + "referenceNode": "a" + }, + { + "name": "fn", + "declaredAt": "Program", + "referenceNode": "fn" + } + ], + "children": [] +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js new file mode 100644 index 000000000..10382e695 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js @@ -0,0 +1,3 @@ +const x = {} +const k = 'y' +x[k].z diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js.snap.json b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js.snap.json new file mode 100644 index 000000000..2cc6ec425 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-computed.js.snap.json @@ -0,0 +1,20 @@ +{ + "type": "Program", + "declarations": [ + "k", + "x" + ], + "references": [ + { + "name": "x", + "declaredAt": "Program", + "referenceNode": "x" + }, + { + "name": "k", + "declaredAt": "Program", + "referenceNode": "k" + } + ], + "children": [] +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js new file mode 100644 index 000000000..d1b7b3331 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js @@ -0,0 +1,8 @@ +const x = {} +x?.y.z + +const a = {} +a.b?.c + +const t = {} +;(t?.u).v diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js.snap.json b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js.snap.json new file mode 100644 index 000000000..a10303fc7 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain-optional.js.snap.json @@ -0,0 +1,26 @@ +{ + "type": "Program", + "declarations": [ + "a", + "t", + "x" + ], + "references": [ + { + "name": "x", + "declaredAt": "Program", + "referenceNode": "x" + }, + { + "name": "a", + "declaredAt": "Program", + "referenceNode": "a.b" + }, + { + "name": "t", + "declaredAt": "Program", + "referenceNode": "t" + } + ], + "children": [] +} diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js new file mode 100644 index 000000000..c8a6f9f60 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js @@ -0,0 +1,2 @@ +const x = {} +x.y.z diff --git a/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js.snap.json b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js.snap.json new file mode 100644 index 000000000..6dc817fb1 --- /dev/null +++ b/packages/plugin-rsc/src/transforms/fixtures/scope/reference-node/member-chain.js.snap.json @@ -0,0 +1,14 @@ +{ + "type": "Program", + "declarations": [ + "x" + ], + "references": [ + { + "name": "x", + "declaredAt": "Program", + "referenceNode": "x.y.z" + } + ], + "children": [] +} diff --git a/packages/plugin-rsc/src/transforms/hoist.test.ts b/packages/plugin-rsc/src/transforms/hoist.test.ts index 2fac98e43..d6b9b642c 100644 --- a/packages/plugin-rsc/src/transforms/hoist.test.ts +++ b/packages/plugin-rsc/src/transforms/hoist.test.ts @@ -1,8 +1,33 @@ +import path from 'node:path' import { parseAstAsync } from 'vite' import { describe, expect, it } from 'vitest' import { transformHoistInlineDirective } from './hoist' import { debugSourceMap } from './test-utils' +describe('fixtures', () => { + const fixtures = import.meta.glob( + ['./fixtures/hoist/**/*.js', '!**/*.snap.js'], + { + query: 'raw', + }, + ) + for (const [file, mod] of Object.entries(fixtures)) { + it(path.basename(file), async () => { + const input = ((await mod()) as any).default as string + const ast = await parseAstAsync(input) + const result = transformHoistInlineDirective(input, ast, { + runtime: (value, name) => + `$$register(${value}, "", ${JSON.stringify(name)})`, + directive: 'use server', + }) + const snapshot = result.output.hasChanged() + ? result.output.toString() + : '/* NO CHANGE */' + await expect(snapshot).toMatchFileSnapshot(file + '.snap.js') + }) + } +}) + describe(transformHoistInlineDirective, () => { async function testTransform( input: string, @@ -875,7 +900,11 @@ function outer() { `) }) - // TODO: check next.js + // TODO: follow up if this edge case matters. + // Next.js's closure-collection logic suggests recursive self-reference is also + // treated as a captured outer name, but we didn't find a direct fixture proving the + // final emitted shape. Our current output self-binds `action`, which is suspicious + // enough to leave as an intentionally-unverified edge case. it('recursion', async () => { const input = `\ function outer() { @@ -932,7 +961,9 @@ function outer() { `) }) - // TODO: check next.js + // Edge case: writing to a captured local only mutates the hoisted action's bound + // parameter copy, not the original outer binding. Next.js appears to have the same + // effective behavior, although via `$$ACTION_ARG_n` rewriting instead of plain params. it('assignment', async () => { const input = ` function Counter() { @@ -965,7 +996,9 @@ function Counter() { `) }) - // TODO + // Same framing as plain assignment above: mutating a captured local is local to the + // hoisted invocation copy. This is probably an unintended edge case rather than a + // behavior worth matching exactly. it('increment', async () => { const input = ` function Counter() { diff --git a/packages/plugin-rsc/src/transforms/hoist.ts b/packages/plugin-rsc/src/transforms/hoist.ts index ad926d1b0..cb1ac7bd5 100644 --- a/packages/plugin-rsc/src/transforms/hoist.ts +++ b/packages/plugin-rsc/src/transforms/hoist.ts @@ -1,4 +1,11 @@ -import type { Program, Literal, Node } from 'estree' +import { tinyassert } from '@hiogawa/utils' +import type { + Program, + Literal, + Node, + MemberExpression, + Identifier, +} from 'estree' import { walk } from 'estree-walker' import MagicString from 'magic-string' import { buildScopeTree, type ScopeTree } from './scope' @@ -68,7 +75,7 @@ export function transformHoistInlineDirective( const bindVars = getBindVars(node, scopeTree) let newParams = [ - ...bindVars, + ...bindVars.map((b) => b.root), ...node.params.map((n) => input.slice(n.start, n.end)), ].join(', ') if (bindVars.length > 0 && options.decode) { @@ -78,7 +85,7 @@ export function transformHoistInlineDirective( ].join(', ') output.appendLeft( node.body.body[0]!.start, - `const [${bindVars.join(',')}] = ${options.decode( + `const [${bindVars.map((b) => b.root).join(',')}] = ${options.decode( '$$hoist_encoded', )};\n`, ) @@ -109,8 +116,8 @@ export function transformHoistInlineDirective( })}` if (bindVars.length > 0) { const bindArgs = options.encode - ? options.encode('[' + bindVars.join(', ') + ']') - : bindVars.join(', ') + ? options.encode('[' + bindVars.map((b) => b.expr).join(', ') + ']') + : bindVars.map((b) => b.expr).join(', ') newCode = `${newCode}.bind(null, ${bindArgs})` } if (declName) { @@ -168,14 +175,149 @@ export function findDirectives(ast: Program, directive: string): Literal[] { return nodes } -function getBindVars(fn: Node, scopeTree: ScopeTree): string[] { +type BindVar = { + root: string // hoisted function param name (root identifier name) + expr: string // bind expression at the call site (root name or synthesized partial object) +} + +// e.g. +// x.y.z -> { key: "y.z", segments: ["y", "z"] } +type BindPath = { + // TODO: This currently models only plain non-computed member chains like + // `x.y.z`. Supporting optional chaining or computed access would require + // richer per-segment metadata and corresponding codegen changes. + key: string + segments: string[] +} + +function getBindVars(fn: Node, scopeTree: ScopeTree): BindVar[] { const fnScope = scopeTree.nodeScope.get(fn)! const ancestorScopes = fnScope.getAncestorScopes() const references = scopeTree.scopeToReferences.get(fnScope) ?? [] - // bind variables that are the ones declared in ancestor scopes but not module global scope + + // bind references that are declared in an ancestor scope, but not module scope nor global const bindReferences = references.filter((id) => { const scope = scopeTree.referenceToDeclaredScope.get(id) return scope && scope !== scopeTree.moduleScope && ancestorScopes.has(scope) }) - return [...new Set(bindReferences.map((id) => id.name))] + + // Group by referenced identifier name (root). + // For each root, track whether the root itself is used + // bare (direct identifier access) or only via member paths. + type IdentifierAccess = + | { kind: 'bare' } + | { kind: 'paths'; paths: BindPath[] } + + const accessMap: Record = {} + + for (const id of bindReferences) { + const name = id.name + const node = scopeTree.referenceToNode.get(id)! + if (node.type === 'Identifier') { + accessMap[name] = { kind: 'bare' } + continue + } + + accessMap[name] ??= { kind: 'paths', paths: [] } + const entry = accessMap[name] + if (entry.kind === 'paths') { + const path = memberExpressionToPath(node) + if (!entry.paths.some((existing) => existing.key === path.key)) { + entry.paths.push(path) + } + } + } + + const result: BindVar[] = [] + for (const [root, entry] of Object.entries(accessMap)) { + if (entry.kind === 'bare') { + result.push({ root, expr: root }) + continue + } + result.push({ + root, + expr: synthesizePartialObject(root, entry.paths), + }) + } + + return result +} + +function memberExpressionToPath(node: MemberExpression): BindPath { + const segments: string[] = [] + let current: Identifier | MemberExpression = node + while (current.type === 'MemberExpression') { + tinyassert(current.property.type === 'Identifier') + segments.unshift(current.property.name) + tinyassert( + current.object.type === 'Identifier' || + current.object.type === 'MemberExpression', + ) + current = current.object + } + return { + key: segments.join('.'), + segments, + } +} + +// Build a nested object literal string from member paths, deduping prefixes +// during trie construction. +// e.g. +// [a, x.y, x.y.z, x.w, s.t] => +// { a: root.a, x: { y: root.x.y, w: root.x.w }, s: { t: root.s.t } } +function synthesizePartialObject(root: string, bindPaths: BindPath[]): string { + type TrieNode = Map + const trie = new Map() + + const paths = dedupeByPrefix(bindPaths.map((p) => p.segments)) + for (const path of paths) { + let node = trie + for (let i = 0; i < path.length; i++) { + const segment = path[i]! + let child = node.get(segment) + if (!child) { + child = new Map() + node.set(segment, child) + } + node = child + } + } + + function serialize(node: TrieNode, segments: string[]): string { + if (node.size === 0) { + return root + segments.map((segment) => `.${segment}`).join('') + } + const entries: string[] = [] + for (const [key, child] of node) { + // ECMAScript object literals treat `__proto__: value` specially: when the + // property name is non-computed and equals "__proto__", evaluation performs + // [[SetPrototypeOf]] instead of creating a normal own data property. Emit a + // computed key here so synthesized partial objects preserve the original + // member-path shape rather than mutating the new object's prototype. + // Spec: https://tc39.es/ecma262/#sec-runtime-semantics-propertydefinitionevaluation + const safeKey = key === '__proto__' ? `["__proto__"]` : key + entries.push(`${safeKey}: ${serialize(child, [...segments, key])}`) + } + return `{ ${entries.join(', ')} }` + } + + return serialize(trie, []) +} + +// e.g. +// [x.y, x.y.z, x.w] -> [x.y, x.w] +// [x.y.z, x.y.z.w] -> [x.y.z] +function dedupeByPrefix(paths: string[][]): string[][] { + const sorted = [...paths].sort((a, b) => a.length - b.length) + const result: string[][] = [] + for (const path of sorted) { + const isPrefix = result.some((existingPath) => + existingPath.every((segment, i) => segment === path[i]), + ) + if (!isPrefix) { + result.push(path) + } + } + return result } diff --git a/packages/plugin-rsc/src/transforms/scope.test.ts b/packages/plugin-rsc/src/transforms/scope.test.ts index 7207772d5..659a8430c 100644 --- a/packages/plugin-rsc/src/transforms/scope.test.ts +++ b/packages/plugin-rsc/src/transforms/scope.test.ts @@ -1,5 +1,5 @@ import path from 'node:path' -import type { Identifier, Node } from 'estree' +import type { Identifier, MemberExpression, Node } from 'estree' import { parseAstAsync } from 'vite' import { describe, expect, it } from 'vitest' import { type Scope, type ScopeTree, buildScopeTree } from './scope' @@ -16,7 +16,8 @@ describe('fixtures', () => { const input = ((await mod()) as any).default as string const ast = await parseAstAsync(input) const scopeTree = buildScopeTree(ast) - const serialized = serializeScopeTree(scopeTree) + const showReferenceNode = file.includes('/scope/reference-node/') + const serialized = serializeScopeTree(scopeTree, { showReferenceNode }) await expect( JSON.stringify(serialized, null, 2) + '\n', ).toMatchFileSnapshot(file + '.snap.json') @@ -29,14 +30,26 @@ describe('fixtures', () => { type SerializedScope = { type: string declarations: string[] - references: { name: string; declaredAt: string | null }[] + references: SerializedReference[] children: SerializedScope[] } -function serializeScopeTree(scopeTree: ScopeTree): SerializedScope { +type SerializedReference = { + name: string + declaredAt: string | null + referenceNode?: string +} + +function serializeScopeTree( + scopeTree: ScopeTree, + options?: { + showReferenceNode?: boolean + }, +): SerializedScope { const { nodeScope, referenceToDeclaredScope, + referenceToNode, scopeToReferences, moduleScope, } = scopeTree @@ -84,14 +97,41 @@ function serializeScopeTree(scopeTree: ScopeTree): SerializedScope { return scopes.map((s) => scopeLabelMap.get(s)!).join(' > ') } + function serializeReferenceNode(node: Identifier | MemberExpression): string { + if (node.type === 'Identifier') { + return node.name + } + // /computed/optional shouldn't show up + // since they aren't collected as reference node yet. + const object = + node.object.type === 'Identifier' || + node.object.type === 'MemberExpression' + ? serializeReferenceNode(node.object) + : '' + const property = + node.property.type === 'Identifier' ? node.property.name : '' + const suffix = node.computed + ? `${node.optional ? '?.' : ''}[${property}]` + : `${node.optional ? '?.' : '.'}${property}` + return object + suffix + } + function serializeScope(scope: Scope): SerializedScope { return { type: scopeLabelMap.get(scope)!, declarations: [...scope.declarations].sort(), - references: getDirectReferences(scope).map((id) => ({ - name: id.name, - declaredAt: serializeDeclaredPath(id), - })), + references: getDirectReferences(scope).map((id) => { + const reference: SerializedReference = { + name: id.name, + declaredAt: serializeDeclaredPath(id), + } + if (options?.showReferenceNode) { + reference.referenceNode = serializeReferenceNode( + referenceToNode.get(id)!, + ) + } + return reference + }), children: scopeChildrenMap .get(scope)! .map((child) => serializeScope(child)), diff --git a/packages/plugin-rsc/src/transforms/scope.ts b/packages/plugin-rsc/src/transforms/scope.ts index 9f57de21d..a02e79b51 100644 --- a/packages/plugin-rsc/src/transforms/scope.ts +++ b/packages/plugin-rsc/src/transforms/scope.ts @@ -1,6 +1,8 @@ +import { tinyassert } from '@hiogawa/utils' import type { Program, Identifier, + MemberExpression, Node, FunctionDeclaration, FunctionExpression, @@ -42,6 +44,10 @@ export type ScopeTree = { // scope-creating AST node → its Scope readonly nodeScope: Map readonly moduleScope: Scope + // each reference Identifier → outermost non-computed MemberExpression rooted at it, or the + // Identifier itself when no such chain exists. Callee position trims the final segment so the + // receiver is captured rather than the method property. + readonly referenceToNode: Map } export function buildScopeTree(ast: Program): ScopeTree { @@ -62,6 +68,7 @@ export function buildScopeTree(ast: Program): ScopeTree { const rawReferences: Array<{ id: Identifier; visitScope: Scope }> = [] const ancestors: Node[] = [] + const referenceToNode = new Map() walk(ast, { enter(node) { @@ -127,18 +134,14 @@ export function buildScopeTree(ast: Program): ScopeTree { } } // Collect reference identifiers for post-walk resolution. - // TODO: - // To extend to member-expression binding: instead of collecting just the - // Identifier, collect the outermost non-computed MemberExpression rooted at - // it (e.g. `x.y` in `x.y.z`) when one exists. The root - // Identifier is still used for `referenceToDeclaredScope`; the full node - // (Identifier | MemberExpression) goes into `scopeToReferences`. Then - // `getBindVars` inspects each entry — if it is a MemberExpression, extract - // the path key for binding instead of the bare name. + // Additionally track member chain for hoist binding. if ( node.type === 'Identifier' && isReferenceIdentifier(node, ancestors.slice(0, -1).reverse()) ) { + const parentStack = ancestors.slice(0, -1).reverse() + const bindableNode = getOutermostBindableReference(node, parentStack) + referenceToNode.set(node, bindableNode) rawReferences.push({ id: node, visitScope: current }) } }, @@ -182,6 +185,7 @@ export function buildScopeTree(ast: Program): ScopeTree { scopeToReferences, nodeScope, moduleScope, + referenceToNode, } } @@ -349,3 +353,45 @@ function isInDestructuringAssignment(parentStack: Node[]): boolean { // targets are owned by an `AssignmentExpression`. return parentStack.some((node) => node.type === 'AssignmentExpression') } + +// Walk up the parent stack collecting non-computed MemberExpression ancestors where the +// current node is the object. Stops at computed access, call boundaries, or any other node. +// In callee position, trims the final segment so we capture the receiver, not the method. +function getOutermostBindableReference( + id: Identifier, + parentStack: Node[], // [direct parent, grandparent, ...] +): Identifier | MemberExpression { + // TODO: This currently accumulates only plain non-computed member chains. + // Supporting optional chaining or computed access would require preserving + // richer access metadata than `MemberExpression` + identifier-name segments. + let current: Identifier | MemberExpression = id + + for (const parent of parentStack) { + if (parent.type === 'MemberExpression' && parent.object === current) { + // Unsupported member hops should conservatively stop at the last safe + // prefix rather than synthesizing a deeper partial object with different + // semantics. + if (parent.computed || parent.optional) { + break + } + current = parent + } else { + // Callee trimming: if we accumulated a member chain and it sits in callee position, + // drop the last segment and capture the receiver instead of the method property. + if ( + parent.type === 'CallExpression' && + parent.callee === current && + current.type === 'MemberExpression' + ) { + tinyassert( + current.object.type === 'Identifier' || + current.object.type === 'MemberExpression', + ) + current = current.object + } + break + } + } + + return current +}