-
Notifications
You must be signed in to change notification settings - Fork 912
feat(cli): fix page file update racing problems and avoid middle state processing #1691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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' | ||
|
|
@@ -9,10 +8,43 @@ import { handlePageChange } from './handlePageChange.js' | |
| import { handlePageUnlink } from './handlePageUnlink.js' | ||
| import { createPageDepsHelper } from './pageDepsHelper.js' | ||
|
|
||
| type PageEventType = 'add' | 'change' | 'unlink' | ||
|
|
||
| /** | ||
| * Merge pending events into final operation. | ||
| */ | ||
| 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' | ||
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
Comment on lines
+11
to
+31
|
||
|
|
||
| /** | ||
| * 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> | ||
| } => { | ||
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // 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>>() | ||
|
|
||
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // watch page deps | ||
| const depsWatcher = chokidar.watch([], { | ||
| ignoreInitial: true, | ||
|
|
@@ -26,13 +58,72 @@ 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 | ||
| await processPageEvents(filePathRelative) | ||
Mister-Hope marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| })() | ||
| 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) | ||
|
|
@@ -77,26 +168,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') | ||
| }) | ||
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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] | ||
| // flush all pending page operations 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()) | ||
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
Mister-Hope marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // clear pending promises | ||
| pagePromises.clear() | ||
|
Comment on lines
+194
to
+201
|
||
| } | ||
|
|
||
| return { | ||
| watchers: [pagesWatcher, depsWatcher], | ||
| cleanup, | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.