fix(solid-router): prevent client effects running on server (#4621)

Author: Ingramz

packages/react-router/src/Matches.tsx

@@ -54,7 +54,7 @@ export function Matches() {
 
   const inner = (
     <ResolvedSuspense fallback={pendingElement}>
-      <Transitioner />
+      {!router.isServer && <Transitioner />}
       <MatchesInner />
     </ResolvedSuspense>
   )

packages/react-router/src/Transitioner.tsx

@@ -30,14 +30,12 @@ export function Transitioner() {
   const isPagePending = isLoading || hasPendingMatches
   const previousIsPagePending = usePrevious(isPagePending)
 
-  if (!router.isServer) {
-    router.startTransition = (fn: () => void) => {
-      setIsTransitioning(true)
-      React.startTransition(() => {
-        fn()
-        setIsTransitioning(false)
-      })
-    }
+  router.startTransition = (fn: () => void) => {
+    setIsTransitioning(true)
+    React.startTransition(() => {
+      fn()
+      setIsTransitioning(false)
+    })
   }
 
   // Subscribe to location changes

packages/solid-router/package.json

@@ -33,7 +33,7 @@
     "test:types:ts56": "node ../../node_modules/typescript56/lib/tsc.js -p tsconfig.legacy.json",
     "test:types:ts57": "node ../../node_modules/typescript57/lib/tsc.js -p tsconfig.legacy.json",
     "test:types:ts58": "tsc -p tsconfig.legacy.json",
-    "test:unit": "vitest",
+    "test:unit": "vitest && vitest --mode server",
     "test:unit:dev": "pnpm run test:unit --watch --hideSkippedTests",
     "test:perf": "vitest bench",
     "test:perf:dev": "pnpm run test:perf --watch --hideSkippedTests",

packages/solid-router/src/Matches.tsx

@@ -50,7 +50,7 @@ export function Matches() {
 
   const inner = (
     <ResolvedSuspense fallback={pendingElement}>
-      <Transitioner />
+      {!router.isServer && <Transitioner />}
       <MatchesInner />
     </ResolvedSuspense>
   )

packages/solid-router/src/Transitioner.tsx

@@ -30,12 +30,10 @@ export function Transitioner() {
   const isPagePending = () => isLoading() || hasPendingMatches()
   const previousIsPagePending = usePrevious(isPagePending)
 
-  if (!router.isServer) {
-    router.startTransition = async (fn: () => void | Promise<void>) => {
-      setIsTransitioning(true)
-      await fn()
-      setIsTransitioning(false)
-    }
+  router.startTransition = async (fn: () => void | Promise<void>) => {
+    setIsTransitioning(true)
+    await fn()
+    setIsTransitioning(false)
   }
 
   // Subscribe to location changes
@@ -66,7 +64,6 @@ export function Transitioner() {
 
   // Try to load the initial location
   Solid.createRenderEffect(() => {
-    if (router.isServer) return
     Solid.untrack(() => {
       if (
         // if we are hydrating from SSR, loading is triggered in ssr-client
@@ -100,6 +97,7 @@ export function Transitioner() {
       },
     ),
   )
+
   Solid.createRenderEffect(
     Solid.on(
       [isPagePending, previousIsPagePending],

packages/solid-router/tests/Transitioner.test.tsx

@@ -10,11 +10,13 @@ import { RouterProvider } from '../src/RouterProvider'
 
 describe('Transitioner', () => {
   it('should call router.load() when Transitioner mounts on the client', async () => {
+    const loader = vi.fn()
     const rootRoute = createRootRoute()
     const indexRoute = createRoute({
       getParentRoute: () => rootRoute,
       path: '/',
       component: () => <div>Index</div>,
+      loader,
     })
 
     const routeTree = rootRoute.addChildren([indexRoute])
@@ -26,43 +28,16 @@ describe('Transitioner', () => {
     })
 
     // Mock router.load() to verify it gets called
-    const loadSpy = vi.spyOn(router, 'load').mockResolvedValue(undefined)
+    const loadSpy = vi.spyOn(router, 'load')
 
-    render(() => <RouterProvider router={router} />)
-
-    // Wait for the createRenderEffect to run and call router.load()
-    await waitFor(() => {
-      expect(loadSpy).toHaveBeenCalledTimes(1)
-    })
-
-    loadSpy.mockRestore()
-  })
-
-  it('should not call router.load() when on the server', async () => {
-    const rootRoute = createRootRoute()
-    const indexRoute = createRoute({
-      getParentRoute: () => rootRoute,
-      path: '/',
-      component: () => <div>Index</div>,
-    })
-
-    const routeTree = rootRoute.addChildren([indexRoute])
-    const router = createRouter({
-      routeTree,
-      history: createMemoryHistory({
-        initialEntries: ['/'],
-      }),
-      isServer: true,
-    })
-
-    // Mock router.load() to verify it gets called
-    const loadSpy = vi.spyOn(router, 'load').mockResolvedValue(undefined)
+    await router.load()
 
     render(() => <RouterProvider router={router} />)
 
     // Wait for the createRenderEffect to run and call router.load()
     await waitFor(() => {
-      expect(loadSpy).toHaveBeenCalledTimes(0)
+      expect(loadSpy).toHaveBeenCalledTimes(2)
+      expect(loader).toHaveBeenCalledTimes(1)
     })
 
     loadSpy.mockRestore()

packages/solid-router/tests/server/Transitioner.test.tsx

@@ -0,0 +1,43 @@
+import { describe, expect, it, vi } from 'vitest'
+import { renderToStringAsync } from 'solid-js/web'
+import {
+  createMemoryHistory,
+  createRootRoute,
+  createRoute,
+  createRouter,
+} from '../../src'
+import { RouterProvider } from '../../src/RouterProvider'
+
+describe('Transitioner (server)', () => {
+  it('should call router.load() only once when on the server', async () => {
+    const loader = vi.fn()
+    const rootRoute = createRootRoute()
+    const indexRoute = createRoute({
+      getParentRoute: () => rootRoute,
+      path: '/',
+      component: () => <div>Index</div>,
+      loader,
+    })
+
+    const routeTree = rootRoute.addChildren([indexRoute])
+    const router = createRouter({
+      routeTree,
+      history: createMemoryHistory({
+        initialEntries: ['/'],
+      }),
+      isServer: true,
+    })
+
+    // Mock router.load() to verify it gets called
+    const loadSpy = vi.spyOn(router, 'load')
+
+    await router.load()
+
+    await renderToStringAsync(() => <RouterProvider router={router} />)
+
+    expect(loadSpy).toHaveBeenCalledTimes(1)
+    expect(loader).toHaveBeenCalledTimes(1)
+
+    loadSpy.mockRestore()
+  })
+})

packages/solid-router/vite.config.ts

@@ -4,22 +4,40 @@ import solid from 'vite-plugin-solid'
 import packageJson from './package.json'
 import type { ViteUserConfig } from 'vitest/config'
 
-const config = defineConfig({
-  plugins: [solid()] as ViteUserConfig['plugins'],
-  test: {
-    name: packageJson.name,
-    dir: './tests',
-    watch: false,
-    environment: 'jsdom',
-    typecheck: { enabled: true },
-    setupFiles: ['./tests/setupTests.tsx'],
-  },
+const config = defineConfig(({ mode }) => {
+  if (mode === 'server') {
+    return {
+      plugins: [solid({ ssr: true })] as ViteUserConfig['plugins'],
+      test: {
+        name: `${packageJson.name} (server)`,
+        dir: './tests/server',
+        watch: false,
+        environment: 'node',
+        typecheck: { enabled: true },
+      },
+    }
+  }
+
+  return {
+    plugins: [solid()] as ViteUserConfig['plugins'],
+    test: {
+      name: packageJson.name,
+      dir: './tests',
+      exclude: ['server'],
+      watch: false,
+      environment: 'jsdom',
+      typecheck: { enabled: true },
+      setupFiles: ['./tests/setupTests.tsx'],
+    },
+  }
 })
 
-export default mergeConfig(
-  config,
-  tanstackViteConfig({
-    entry: ['./src/index.tsx', './src/ssr/client.ts', './src/ssr/server.ts'],
-    srcDir: './src',
-  }),
+export default defineConfig((env) =>
+  mergeConfig(
+    config(env),
+    tanstackViteConfig({
+      entry: ['./src/index.tsx', './src/ssr/client.ts', './src/ssr/server.ts'],
+      srcDir: './src',
+    }),
+  ),
 )