fix(router): avoid route watcher rebuild churn

Author: natew

apps/onestack.dev/app/routes.d.ts

@@ -6,7 +6,7 @@ import type { OneRouter } from 'one'
 declare module 'one' {
   export namespace OneRouter {
     export interface __routes<T extends string = string> extends Record<string, unknown> {
-      StaticRoutes: 
+      StaticRoutes:
         | `/`
         | `/(content)`
         | `/(content)/blog`

examples/one-basic/app/routes.d.ts

@@ -6,7 +6,7 @@ import type { OneRouter } from 'one'
 declare module 'one' {
   export namespace OneRouter {
     export interface __routes<T extends string = string> extends Record<string, unknown> {
-      StaticRoutes: 
+      StaticRoutes:
         | `/`
         | `/_sitemap`
         | `/tabs`

packages/one/src/typed-routes/generateRouteTypes.test.ts

@@ -0,0 +1,53 @@
+import {
+  mkdirSync,
+  mkdtempSync,
+  rmSync,
+  statSync,
+  utimesSync,
+  writeFileSync,
+} from 'node:fs'
+import { tmpdir } from 'node:os'
+import path from 'node:path'
+import { afterEach, describe, expect, it } from 'vitest'
+import { generateRouteTypes } from './generateRouteTypes'
+
+describe(generateRouteTypes, () => {
+  const originalCwd = process.cwd()
+  let tempRoot: string | undefined
+
+  afterEach(() => {
+    process.chdir(originalCwd)
+
+    if (tempRoot) {
+      rmSync(tempRoot, { recursive: true, force: true })
+      tempRoot = undefined
+    }
+  })
+
+  it('does not rewrite routes declarations when contents are unchanged', async () => {
+    tempRoot = mkdtempSync(path.join(tmpdir(), 'one-routes-types-'))
+    const appDir = path.join(tempRoot, 'app')
+    mkdirSync(appDir)
+    writeFileSync(
+      path.join(appDir, 'index+ssg.tsx'),
+      'export default function Index() { return null }\n'
+    )
+    writeFileSync(
+      path.join(appDir, '[slug]+ssg.tsx'),
+      'export default function Slug() { return null }\n'
+    )
+
+    process.chdir(tempRoot)
+
+    const outFile = path.join('app', 'routes.d.ts')
+    await generateRouteTypes(outFile, 'app')
+
+    const oldDate = new Date('2001-01-01T00:00:00.000Z')
+    utimesSync(outFile, oldDate, oldDate)
+    const previousMtimeMs = statSync(outFile).mtimeMs
+
+    await generateRouteTypes(outFile, 'app')
+
+    expect(statSync(outFile).mtimeMs).toBe(previousMtimeMs)
+  })
+})

packages/one/src/typed-routes/generateRouteTypes.ts

@@ -1,4 +1,4 @@
-import { writeFile } from 'node:fs/promises'
+import { readFile, writeFile } from 'node:fs/promises'
 import { dirname, join } from 'node:path'
 import FSExtra from 'fs-extra'
 import micromatch from 'micromatch'
@@ -33,8 +33,19 @@ export async function generateRouteTypes(
   const context = globbedRoutesToRouteContext(routes, routerRoot)
   const declarations = getTypedRoutesDeclarationFile(context)
   const outDir = dirname(outFile)
-  await FSExtra.ensureDir(outDir)
-  await writeFile(outFile, declarations)
+  let currentDeclarations: string | undefined
+  try {
+    currentDeclarations = await readFile(outFile, 'utf8')
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+      throw error
+    }
+  }
+
+  if (currentDeclarations !== declarations) {
+    await FSExtra.ensureDir(outDir)
+    await writeFile(outFile, declarations)
+  }
 
   // If experimental.typedRoutesGeneration is enabled, inject helpers into route files
   if (typedRoutesMode) {

packages/one/src/typed-routes/getTypedRoutesDeclarationFile.test.ts

@@ -0,0 +1,22 @@
+import { describe, expect, it } from 'vitest'
+import type { One } from '../vite/types'
+import { getTypedRoutesDeclarationFile } from './getTypedRoutesDeclarationFile'
+
+function createRouteContext(paths: string[]) {
+  const context = (() => ({ default() {} })) as unknown as One.RouteContext
+  Object.defineProperty(context, 'keys', {
+    value: () => paths,
+  })
+  return context
+}
+
+describe(getTypedRoutesDeclarationFile, () => {
+  it('does not emit trailing whitespace for multi-line route unions', () => {
+    const declaration = getTypedRoutesDeclarationFile(
+      createRouteContext(['./index+ssg.tsx', './about+ssg.tsx', './[slug]+ssg.tsx'])
+    )
+
+    expect(declaration).toContain('      StaticRoutes:\n        | `/`')
+    expect(declaration).not.toMatch(/[ \t]+$/m)
+  })
+})

packages/one/src/typed-routes/getTypedRoutesDeclarationFile.ts

@@ -36,9 +36,9 @@ import type { OneRouter } from 'one'
 declare module 'one' {
   export namespace OneRouter {
     export interface __routes<T extends string = string> extends Record<string, unknown> {
-      StaticRoutes: ${setToUnionType(staticRoutes)}
-      DynamicRoutes: ${setToUnionType(dynamicRoutes)}
-      DynamicRouteTemplate: ${setToUnionType(dynamicRouteContextKeys)}
+      StaticRoutes:${setToUnionType(staticRoutes)}
+      DynamicRoutes:${setToUnionType(dynamicRoutes)}
+      DynamicRouteTemplate:${setToUnionType(dynamicRouteContextKeys)}
       IsTyped: true
       ${hasRoutes ? `RouteTypes: ${generateRouteTypesMap(dynamicRouteContextKeys)}` : ''}
     }
@@ -189,9 +189,9 @@ function addRouteNode(
  * Formats with one route per line for cleaner git diffs
  */
 const setToUnionType = <T>(set: Set<T>) => {
-  if (set.size === 0) return 'never'
+  if (set.size === 0) return ' never'
   const sorted = [...set].sort()
-  if (sorted.length === 1) return `\`${sorted[0]}\``
+  if (sorted.length === 1) return ` \`${sorted[0]}\``
   // format as multi-line union for cleaner diffs
   return '\n        | ' + sorted.map((s) => `\`${s}\``).join('\n        | ')
 }

packages/one/src/utils/routeFileWatch.test.ts

@@ -0,0 +1,94 @@
+import path from 'node:path'
+import { describe, expect, it } from 'vitest'
+import {
+  isPathInsideDirectory,
+  isRouteFilePath,
+  isRouteFileWatchEvent,
+} from './routeFileWatch'
+
+describe(isRouteFilePath, () => {
+  it('matches route source files', () => {
+    expect(isRouteFilePath('/project/app/index.ts')).toBe(true)
+    expect(isRouteFilePath('/project/app/index.tsx')).toBe(true)
+    expect(isRouteFilePath('/project/app/index.js')).toBe(true)
+    expect(isRouteFilePath('/project/app/index.jsx')).toBe(true)
+  })
+
+  it('ignores non-route files', () => {
+    expect(isRouteFilePath('/project/app/tamagui.generated.css')).toBe(false)
+    expect(isRouteFilePath('/project/app/routes.d.ts')).toBe(false)
+  })
+})
+
+describe(isPathInsideDirectory, () => {
+  it('only matches real descendants of the router root', () => {
+    const routerRoot = path.resolve('/project/app')
+
+    expect(isPathInsideDirectory('/project/app/index.tsx', routerRoot)).toBe(true)
+    expect(isPathInsideDirectory('/project/app-copy/index.tsx', routerRoot)).toBe(false)
+    expect(isPathInsideDirectory('/project/app', routerRoot)).toBe(false)
+  })
+})
+
+describe(isRouteFileWatchEvent, () => {
+  const routerRoot = path.resolve('/project/app')
+
+  it('matches route file add and delete events', () => {
+    expect(
+      isRouteFileWatchEvent({
+        event: 'add',
+        filePath: '/project/app/index.tsx',
+        routerRoot,
+      })
+    ).toBe(true)
+    expect(
+      isRouteFileWatchEvent({
+        event: 'delete',
+        filePath: '/project/app/nested/page.jsx',
+        routerRoot,
+      })
+    ).toBe(true)
+    expect(
+      isRouteFileWatchEvent({
+        event: 'unlink',
+        filePath: '/project/app/nested/page.jsx',
+        routerRoot,
+      })
+    ).toBe(true)
+  })
+
+  it('ignores non-route file add and delete events', () => {
+    expect(
+      isRouteFileWatchEvent({
+        event: 'add',
+        filePath: '/project/app/tamagui.generated.css',
+        routerRoot,
+      })
+    ).toBe(false)
+    expect(
+      isRouteFileWatchEvent({
+        event: 'delete',
+        filePath: '/project/app/routes.d.ts',
+        routerRoot,
+      })
+    ).toBe(false)
+  })
+
+  it('only matches change events when requested', () => {
+    expect(
+      isRouteFileWatchEvent({
+        event: 'change',
+        filePath: '/project/app/index.tsx',
+        routerRoot,
+      })
+    ).toBe(false)
+    expect(
+      isRouteFileWatchEvent({
+        event: 'change',
+        filePath: '/project/app/index.tsx',
+        routerRoot,
+        includeChangeEvents: true,
+      })
+    ).toBe(true)
+  })
+})

packages/one/src/utils/routeFileWatch.ts

@@ -0,0 +1,40 @@
+import path from 'node:path'
+
+const routeFileExtensionRe = /\.[jt]sx?$/
+
+export function isRouteFilePath(filePath: string) {
+  return routeFileExtensionRe.test(filePath) && !filePath.endsWith('.d.ts')
+}
+
+export function isPathInsideDirectory(filePath: string, directory: string) {
+  const relativePath = path.relative(path.resolve(directory), path.resolve(filePath))
+  return (
+    relativePath !== '' &&
+    !relativePath.startsWith('..') &&
+    !path.isAbsolute(relativePath)
+  )
+}
+
+export function isRouteFileWatchEvent({
+  event,
+  filePath,
+  routerRoot,
+  includeChangeEvents = false,
+}: {
+  event: string
+  filePath: string
+  routerRoot: string
+  includeChangeEvents?: boolean
+}) {
+  const isRouteFileEvent =
+    event === 'add' ||
+    event === 'delete' ||
+    event === 'unlink' ||
+    (includeChangeEvents && event === 'change')
+
+  return (
+    isRouteFileEvent &&
+    isPathInsideDirectory(filePath, routerRoot) &&
+    isRouteFilePath(filePath)
+  )
+}

packages/one/src/vite/plugins/fileSystemRouterPlugin.test.ts

@@ -0,0 +1,106 @@
+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import path from 'node:path'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('vite', async () => {
+  const actual = await vi.importActual<typeof import('vite')>('vite')
+  return {
+    ...actual,
+    createServerModuleRunner: vi.fn(() => ({
+      clearCache: vi.fn(),
+      import: vi.fn(),
+    })),
+  }
+})
+
+describe('createFileSystemRouterPlugin', () => {
+  const previousIsVxrnCli = process.env.IS_VXRN_CLI
+  let previousVxrnPluginConfig: unknown
+  let tempRoot: string | undefined
+
+  beforeEach(() => {
+    previousVxrnPluginConfig = (globalThis as any).__vxrnPluginConfig__
+  })
+
+  afterEach(() => {
+    if (tempRoot) {
+      rmSync(tempRoot, { recursive: true, force: true })
+      tempRoot = undefined
+    }
+
+    if (previousIsVxrnCli === undefined) {
+      delete process.env.IS_VXRN_CLI
+    } else {
+      process.env.IS_VXRN_CLI = previousIsVxrnCli
+    }
+
+    if (previousVxrnPluginConfig === undefined) {
+      delete (globalThis as any).__vxrnPluginConfig__
+    } else {
+      ;(globalThis as any).__vxrnPluginConfig__ = previousVxrnPluginConfig
+    }
+    vi.restoreAllMocks()
+  })
+
+  it('keeps route watcher rebuild errors handled', async () => {
+    process.env.IS_VXRN_CLI = '1'
+    tempRoot = mkdtempSync(path.join(tmpdir(), 'one-router-watch-'))
+    const appDir = path.join(tempRoot, 'app')
+    writeFileSync(path.join(tempRoot, 'package.json'), '{}\n')
+    mkdirSync(appDir)
+    writeFileSync(
+      path.join(appDir, 'index.tsx'),
+      'export default function Index() { return null }\n'
+    )
+
+    ;(globalThis as any).__vxrnPluginConfig__ = {
+      web: {
+        defaultRenderMode: 'ssg',
+      },
+    }
+
+    const { createFileSystemRouterPlugin } = await import('./fileSystemRouterPlugin')
+    const plugin = createFileSystemRouterPlugin({
+      router: {
+        root: appDir,
+      },
+    })
+
+    let watcherListener:
+      | ((event: string, changedPath: string) => void | Promise<void>)
+      | undefined
+    const server = {
+      environments: {
+        ssr: {},
+      },
+      watcher: {
+        addListener: vi.fn((event: string, listener: typeof watcherListener) => {
+          if (event === 'all') {
+            watcherListener = listener
+          }
+        }),
+        on: vi.fn(),
+      },
+    }
+
+    ;(plugin as any).configureServer(server)
+    expect(watcherListener).toBeDefined()
+
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    delete (globalThis as any).__vxrnPluginConfig__
+
+    if (!watcherListener) {
+      throw new Error('Expected route watcher listener to be registered')
+    }
+
+    await expect(
+      Promise.resolve(watcherListener('add', path.join(appDir, 'new-route.tsx')))
+    ).resolves.toBeUndefined()
+
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('[one] Failed to rebuild routes'),
+      expect.any(Error)
+    )
+  })
+})