nksaraf / vinxi

The Full Stack JavaScript SDK
https://vinxi.vercel.app
MIT License
1.33k stars 57 forks source link

Outer variables referenced inside "use server" get treeshaken away. #296

Open lithdew opened 1 month ago

lithdew commented 1 month ago

Reproduction: https://stackblitz.com/edit/github-hvdm3o?file=app%2Fserver-functions.ts

The reproduction is a fork of examples/vanilla/spa.

Error:

[h3] [unhandled] ReferenceError: updatedSchema is not defined
    at $$function0 (/home/projects/kkvirzoaz.github/app/server-functions.ts:32:22)
    ... 3 lines matching cause stack trace ...
    at async Server.toNodeHandle (file:///home/projects/kkvirzoaz.github/node_modules/h3/dist/index.mjs:2374:7) {
  cause: ReferenceError: updatedSchema is not defined
      at $$function0 (/home/projects/kkvirzoaz.github/app/server-functions.ts:32:22)
      at handleServerAction (/home/projects/kkvirzoaz.github/node_modules/@vinxi/server-functions/server-handler.js:17:27)
      at async _callHandler (file:///home/projects/kkvirzoaz.github/node_modules/h3/dist/index.mjs:1946:16)
      at async Object.eval [as handler] (file:///home/projects/kkvirzoaz.github/node_modules/h3/dist/index.mjs:2087:19)
      at async Server.toNodeHandle (file:///home/projects/kkvirzoaz.github/node_modules/h3/dist/index.mjs:2374:7),
  statusCode: 500,
  fatal: false,
  unhandled: true,
  statusMessage: undefined,
  data: undefined
}

The function world shown below is the server function. The updatedSchema variable declared inside validated gets treeshaken away even though it is used inside of the "use server" closure.

import { z } from 'zod';

export class ValidationError extends Error {
  constructor(public errors: z.ZodIssue[]) {
    super('Failed to validate input.');
  }
}

export function validated<TInputSchema extends z.ZodSchema, T>(
  schema: TInputSchema,
  fn: (input: z.input<TInputSchema>) => T
) {
  const updatedSchema = schema.transform((v) => ({
    status: 'ok' as const,
    data: v,
  }));
  return (input: z.input<TInputSchema>) => {
    'use server';
    const validated = updatedSchema.safeParse(input);
    if (!validated.success) {
      throw new ValidationError(validated.error.errors);
    }

    return fn(validated.data);
  };
}

export const world = validated(
  z.object({ text: z.string() }),
  (input) => input
);

The client.tsx is modified as shown below:

/// <reference types="vinxi/types/client" />
import 'vinxi/client';

import './style.css';
import { world } from './server-functions';

world({ text: 'world' }).then(
  (result) => (document.getElementById('app').innerHTML = `Hello ${result}`)
);
Brendonovich commented 1 month ago

Though the error reporting could be improved, I think this is intended behaviour. Functions with 'use server' are hoisted to the module scope and can't capture things that aren't also available in the module.

lithdew commented 1 month ago

🤔 For NextJS’s server actions implementation as an example, the “use server” directive can only be used in server-side files though (whether or not the directive is declared at the top of a module or function). Implicitly, this would mean files with “use server” declared anywhere should be excluded from the client bundle (well, included in the client bundle with a shim based on createServerReference).

I noticed that for Vinxi’s server functions that “use server” can technically be declared in functions in client-side code as well (causing client-side and server-side code to mix). It’s not that trivial to resolve which imports or functions inside of mixed client/server code should be included solely in the client bundle or server bundle in that case though. Is this intended behavior?

Brendonovich commented 1 month ago

I think it is intended behaviour yeah, since Vinxi doesn't have a separation between client and server files like React.

It’s not that trivial to resolve which imports or functions inside of mixed client/server code should be included solely in the client bundle or server bundle in that case though

I think something that would help here is for server functions to only be valid at the top-level of a module, such that the code you provided doesn't compile successfully. That way it wouldn't be possible for the server function to close over updatedSchema.