feat(cli): fix page file update racing problems and avoid middle state processing (#1691)

Author: Mister-Hope

packages/cli/src/commands/dev/dev.ts

@@ -66,6 +66,10 @@ export const dev: DevCommand = async ({
   // all watchers
   const watchers: FSWatcher[] = []
 
+  const { watchers: pageWatchers, cleanup: pageCleanup } = watchPageFiles(app)
+  // watch page files
+  watchers.push(...pageWatchers)
+
   // restart dev command
   const restart = async (): Promise<void> => {
     await Promise.all([
@@ -74,7 +78,8 @@ export const dev: DevCommand = async ({
       // close current dev server
       close(),
     ])
-
+    // flush pending page file operations and cleanup
+    await pageCleanup()
     // clean up internal app state after all watchers and server are closed
     app.writeTemp.cleanup()
 
@@ -92,9 +97,6 @@ export const dev: DevCommand = async ({
     logger.tip(`dev server has restarted, please refresh your browser`)
   }
 
-  // watch page files
-  watchers.push(...watchPageFiles(app))
-
   // watch user config file
   if (userConfigPath) {
     watchers.push(

packages/cli/src/commands/dev/watchPageFiles.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/no-misused-promises */
 import type { App, Page } from '@vuepress/core'
 import { colors, logger, path, picomatch } from '@vuepress/utils'
 import type { FSWatcher } from 'chokidar'
@@ -10,10 +9,44 @@ import { handlePageUnlink } from './handlePageUnlink.js'
 import { createPageDepsHelper } from './pageDepsHelper.js'
 import { processPagePatterns } from './processPagePatterns.js'
 
+type PageEventType = 'add' | 'change' | 'unlink'
+
+/**
+ * Merge pending events into final operation.
+ * Exported for testing purposes.
+ */
+export const mergeEvents = (events: PageEventType[]): PageEventType | null => {
+  if (events.length === 0) return null
+
+  if (events.length === 1) return events[0]
+
+  const first = events[0]
+  const last = events[events.length - 1]
+
+  // add + ... + remove: nothing
+  if (first === 'add' && last === 'unlink') return null
+
+  if (first === 'add') return 'add'
+  if (last === 'unlink') return 'unlink'
+
+  return 'change'
+}
+
 /**
- * Watch page files and deps, return file watchers
+ * Watch page files and deps, return file watchers and cleanup function
  */
-export const watchPageFiles = (app: App): FSWatcher[] => {
+export const watchPageFiles = (
+  app: App,
+): {
+  watchers: FSWatcher[]
+  cleanup: () => Promise<void>
+} => {
+  // Track pending events per page - just event types, no I/O
+  const pendingEvents = new Map<string, PageEventType[]>()
+
+  // Track the last promise per page for serialization
+  const pagePromises = new Map<string, Promise<void>>()
+
   // watch page deps
   const depsWatcher = chokidar.watch([], {
     ignoreInitial: true,
@@ -27,13 +60,84 @@ export const watchPageFiles = (app: App): FSWatcher[] => {
     const depsToRemove = depsHelper.remove(page)
     depsWatcher.unwatch(depsToRemove)
   }
-  const depsListener = async (dep: string): Promise<void> => {
+
+  // Process pending events for a page, merging them into one final operation
+  const processPageEvents = async (filePathRelative: string): Promise<void> => {
+    // Get and clear pending events for this page
+    const events = pendingEvents.get(filePathRelative) ?? []
+    pendingEvents.delete(filePathRelative)
+
+    // Merge events into final operation
+    const finalEvent = mergeEvents(events)
+    if (!finalEvent) return
+
+    const filePath = app.dir.source(filePathRelative)
+
+    if (finalEvent === 'add') {
+      logger.info(`page ${colors.magenta(filePathRelative)} is created`)
+      const page = await handlePageAdd(app, filePath)
+      if (page === null) return
+      addDeps(page)
+      return
+    }
+
+    if (finalEvent === 'change') {
+      logger.info(`page ${colors.magenta(filePathRelative)} is modified`)
+      const result = await handlePageChange(app, filePath)
+      if (result === null) return
+      const [pageOld, pageNew] = result
+      removeDeps(pageOld)
+      addDeps(pageNew)
+      return
+    }
+
+    // finalEvent is 'unlink'
+    logger.info(`page ${colors.magenta(filePathRelative)} is removed`)
+    const page = await handlePageUnlink(app, filePath)
+    if (page === null) return
+    removeDeps(page)
+  }
+
+  // Handle file events - just track them, no processing yet
+  const pageEventHandler = (
+    filePathRelative: string,
+    eventType: PageEventType,
+  ): void => {
+    // Add event to pending list
+    let events = pendingEvents.get(filePathRelative)
+    if (!events) pendingEvents.set(filePathRelative, (events = []))
+    events.push(eventType)
+
+    // Chain to existing promise to ensure serialization
+    const existingPromise =
+      pagePromises.get(filePathRelative) ?? Promise.resolve()
+    const newPromise = (async () => {
+      await existingPromise
+      try {
+        await processPageEvents(filePathRelative)
+      } catch (error) {
+        logger.error(
+          `Error while processing page events for ${colors.magenta(filePathRelative)}:`,
+          error,
+        )
+      }
+    })()
+      // Only delete if this promise is still the current one (compare by identity)
+      .finally(() => {
+        if (pagePromises.get(filePathRelative) === newPromise)
+          pagePromises.delete(filePathRelative)
+      })
+    pagePromises.set(filePathRelative, newPromise)
+  }
+
+  // When a dependency changes, find all pages that depend on it and trigger change event for them
+  const depsListener = (dep: string): void => {
     const pagePaths = depsHelper.get(dep)
     for (const filePathRelative of pagePaths) {
       logger.info(
         `dependency of page ${colors.magenta(filePathRelative)} is modified`,
       )
-      await handlePageChange(app, app.dir.source(filePathRelative))
+      pageEventHandler(filePathRelative, 'change')
     }
   }
   depsWatcher.on('add', depsListener)
@@ -72,26 +176,29 @@ export const watchPageFiles = (app: App): FSWatcher[] => {
     },
     ignoreInitial: true,
   })
-  pagesWatcher.on('add', async (filePathRelative) => {
-    logger.info(`page ${colors.magenta(filePathRelative)} is created`)
-    const page = await handlePageAdd(app, app.dir.source(filePathRelative))
-    if (page === null) return
-    addDeps(page)
+
+  pagesWatcher.on('add', (filePathRelative) => {
+    pageEventHandler(filePathRelative, 'add')
   })
-  pagesWatcher.on('change', async (filePathRelative) => {
-    logger.info(`page ${colors.magenta(filePathRelative)} is modified`)
-    const result = await handlePageChange(app, app.dir.source(filePathRelative))
-    if (result === null) return
-    const [pageOld, pageNew] = result
-    removeDeps(pageOld)
-    addDeps(pageNew)
+  pagesWatcher.on('change', (filePathRelative) => {
+    pageEventHandler(filePathRelative, 'change')
   })
-  pagesWatcher.on('unlink', async (filePathRelative) => {
-    logger.info(`page ${colors.magenta(filePathRelative)} is removed`)
-    const page = await handlePageUnlink(app, app.dir.source(filePathRelative))
-    if (page === null) return
-    removeDeps(page)
+  pagesWatcher.on('unlink', (filePathRelative) => {
+    pageEventHandler(filePathRelative, 'unlink')
   })
 
-  return [pagesWatcher, depsWatcher]
+  // cancel queued page events, wait for in-flight operations to finish, and reset
+  const cleanup = async (): Promise<void> => {
+    // clear pending events
+    pendingEvents.clear()
+    // wait for all pending page operations to finish
+    await Promise.all(pagePromises.values())
+    // clear pending promises
+    pagePromises.clear()
+  }
+
+  return {
+    watchers: [pagesWatcher, depsWatcher],
+    cleanup,
+  }
 }

packages/cli/tests/commands/dev/watchPageFiles.spec.ts

@@ -0,0 +1,65 @@
+import { describe, expect, it } from 'vitest'
+
+import { mergeEvents } from '../../../src/commands/dev/watchPageFiles.js'
+
+describe('mergeEvents', () => {
+  it('should return null for empty events', () => {
+    expect(mergeEvents([])).toBe(null)
+  })
+
+  describe('single event', () => {
+    it('should return "add" for a single add event', () => {
+      expect(mergeEvents(['add'])).toBe('add')
+    })
+
+    it('should return "change" for a single change event', () => {
+      expect(mergeEvents(['change'])).toBe('change')
+    })
+
+    it('should return "unlink" for a single unlink event', () => {
+      expect(mergeEvents(['unlink'])).toBe('unlink')
+    })
+  })
+
+  describe('add → ...', () => {
+    it('should merge add + change into "add" (file added then modified before processing)', () => {
+      expect(mergeEvents(['add', 'change'])).toBe('add')
+    })
+
+    it('should merge add + change + change into "add"', () => {
+      expect(mergeEvents(['add', 'change', 'change'])).toBe('add')
+    })
+
+    it('should merge add + unlink into null (file added then removed, nothing happened)', () => {
+      expect(mergeEvents(['add', 'unlink'])).toBe(null)
+    })
+
+    it('should merge add + change + unlink into null', () => {
+      expect(mergeEvents(['add', 'change', 'unlink'])).toBe(null)
+    })
+  })
+
+  describe('unlink → ...', () => {
+    it('should merge unlink + add into "change" (atomic save: remove then re-add equals a change)', () => {
+      expect(mergeEvents(['unlink', 'add'])).toBe('change')
+    })
+
+    it('should merge unlink + add + change into "change"', () => {
+      expect(mergeEvents(['unlink', 'add', 'change'])).toBe('change')
+    })
+  })
+
+  describe('change → ...', () => {
+    it('should merge change + change into "change"', () => {
+      expect(mergeEvents(['change', 'change'])).toBe('change')
+    })
+
+    it('should merge change + unlink into "unlink"', () => {
+      expect(mergeEvents(['change', 'unlink'])).toBe('unlink')
+    })
+
+    it('should merge change + change + change into "change"', () => {
+      expect(mergeEvents(['change', 'change', 'change'])).toBe('change')
+    })
+  })
+})