TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
8.26k stars 654 forks source link

Start - Server Function Middleware Included in Client Bundle #2783

Open scrawfor opened 6 days ago

scrawfor commented 6 days ago

Which project does this relate to?

Start

Describe the bug

Calling a serverFn using middleware in the client with useServerFn results in server side code being included in client bundle.

I'll try to get a minimal reproduction later this weekend or early next week. But here is an example of the issue.

Your Example Website or App

StackBlitz Reproduction Link

Steps to Reproduce the Bug or Issue

Basic Example:

import { getSupabaseServerClient } from "@/features/supabase/utils";
import { createServerFn, createMiddleware } from "@tanstack/start";
import { useQuery } from "@tanstack/react-query";
import { z } from 'zod'

export const authMiddleware = createMiddleware().server(async ({ next }) => {
  const supa = getSupabaseServerClient();
  const user = await supa.auth.getUser();
  return next({ context: { user } });
});

const serverFilters = z.object({ })

const getQuery = createServerFn({ method: "GET" })
  .middleware([authMiddleware])
  .validator((input: unknown) => serverFilters.parse(input))
  .handler(async ({ data, context }) => {
    ...
  });

function useQueryData(data: any) {
    const getData = useServerFn(getQuery);
    return useQuery({
      queryKey: ["id", JSON.stringify(filters)],
      queryFn: () => getData({ data: filters }),
    });

}

Client Side Error: I see a request for the file "@/features/supabase/utils", and get this console error.

Uncaught Error: Module "node:async_hooks" has been externalized for browser compatibility. Cannot access "node:async_hooks.AsyncLocalStorage" in client code.  See https://vite.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.
    get __vite-browser-external:node:async_hooks:3
    <anonymous> http.js:71
[__vite-browser-external:node:async_hooks:3:10](http://localhost:3000/_build/@id/__vite-browser-external:node:async_hooks)

Expected behavior

No error & the file "@/features/supabase/utils" is not included in client side code.

Removing the middleware from the server function resolves the issue. The same reference to getSupbaseServerClient inside the serverFn handler runs without issue.

Screenshots or Videos

No response

Platform

Additional context

No response

SeanCassiere commented 5 days ago

From a cursory overview, it seems that dead-code-elimination isn't being performed on the client references code before being packaged into their necessary bundles.

https://github.com/TanStack/router/blob/55891518fcfa7c308e1db4149d357cf38700c882/packages/start-vite-plugin/tests/createMiddleware/snapshots/client/createMiddlewareDestructured.tsx#L12-L21

dotnize commented 5 days ago

What worked for me was moving the middleware definition (createMiddleware) into its own separate file

SeanCassiere commented 4 days ago

I'm having difficult reproducing this using our "start-supabase-basic" example.

import * as React from 'react'
import { createFileRoute } from '@tanstack/react-router'
import { createMiddleware, createServerFn, useServerFn } from '@tanstack/start'
import { getSupabaseServerClient } from '~/utils/supabase'

const authMiddleware = createMiddleware().server(async ({ next }) => {
  const supa = getSupabaseServerClient()
  const user = await supa.auth.getUser()
  console.log('user response', user)
  return next({ context: { user } })
})

const checkAuthFn = createServerFn()
  .middleware([authMiddleware])
  .handler(({ context }) => {
    console.log('checkAuthFn', context.user)
    return { foo: 'bar' }
  })

export const Route = createFileRoute('/test')({
  component: RouteComponent,
})

function RouteComponent() {
  const fn = useServerFn(checkAuthFn)

  return (
    <div className="flex gap-2">
      <button
        onClick={() => {
          checkAuthFn().then((res) => {
            console.log(Date.now(), 'direct', res)
          })
        }}
      >
        Test Direct
      </button>
      <button
        onClick={() => {
          fn().then((res) => {
            console.log(Date.now(), 'useServerFn', res)
          })
        }}
      >
        Test useServerFn
      </button>
    </div>
  )
}
scrawfor commented 4 days ago

@SeanCassiere - I'll try to pull together a reproduction tomorrow. I attempted yesterday but I was having issues with StackBlitz.

I will say my example was overly simplistic. I actually had the Middleware in a separate file. Tonight I tried defining the middleware in the same file as the server function and that worked. The contents of the middleware were the same in both cases.

I'm having difficult reproducing this using our "start-supabase-basic" example.

scrawfor commented 4 days ago

@SeanCassiere - I've created a reproduction on StackBlitz. This example handles it more gracefully than my actual case (where the route crashes and burns), but it still reproduces. If you open your browser dev tools, the console should show node-related errors and also load server side packages

I reproduced it with two separate middleware functions using two separate node packages. I added the server function with middleware to app/routes/index.tsx.

First Reproduction

The first is using supabase as mentioned above. features/supabase.ts creates a supbase client and an authMiddleware. This authMiddleware doesn't actually do anything, but it's defined in the same file as the supabase code. Importing it will cause the errors mentioned above.

Second Reproduction

The second reproduction is in features/db.ts. It defines a dbMiddleware that adds a clickhouse client to the context. Uncomment .middleware([dbMiddlware]) from app/routes/index.tsx. This results in a slightly different console error, but the root cause is the same. The Clickhouse client is pulled into client side code. If the same dbMilddleware code is instead defined in app/routes/index this doesn't break.