From 24727af4e848ca355d2685e850905575393ac235 Mon Sep 17 00:00:00 2001 From: Divyansh Singh <40380293+brc-dd@users.noreply.github.com> Date: Mon, 24 Feb 2025 18:57:51 +0530 Subject: [PATCH] fix --- __tests__/e2e/dynamic-routes/[id].paths.ts | 2 +- __tests__/unit/node/utils/moduleGraph.test.ts | 72 +++++ src/node/plugins/dynamicRoutesPlugin.ts | 258 +++++++++--------- src/node/utils/moduleGraph.ts | 115 ++++++++ 4 files changed, 322 insertions(+), 125 deletions(-) create mode 100644 __tests__/unit/node/utils/moduleGraph.test.ts create mode 100644 src/node/utils/moduleGraph.ts diff --git a/__tests__/e2e/dynamic-routes/[id].paths.ts b/__tests__/e2e/dynamic-routes/[id].paths.ts index 48e35261..2874313f 100644 --- a/__tests__/e2e/dynamic-routes/[id].paths.ts +++ b/__tests__/e2e/dynamic-routes/[id].paths.ts @@ -2,7 +2,7 @@ import paths from './paths' export default { async paths(watchedFiles: string[]) { - console.log('watchedFiles', watchedFiles) + // console.log('watchedFiles', watchedFiles) return paths }, watch: ['**/data-loading/**/*.json'] diff --git a/__tests__/unit/node/utils/moduleGraph.test.ts b/__tests__/unit/node/utils/moduleGraph.test.ts new file mode 100644 index 00000000..9b3db9e5 --- /dev/null +++ b/__tests__/unit/node/utils/moduleGraph.test.ts @@ -0,0 +1,72 @@ +import { ModuleGraph } from 'node/utils/moduleGraph' + +describe('node/utils/moduleGraph', () => { + let graph: ModuleGraph + + beforeEach(() => { + graph = new ModuleGraph() + }) + + it('should correctly delete a module and its dependents', () => { + graph.add('A', ['B', 'C']) + graph.add('B', ['D']) + graph.add('C', []) + graph.add('D', []) + + expect(graph.delete('D')).toEqual(new Set(['D', 'B', 'A'])) + }) + + it('should handle shared dependencies correctly', () => { + graph.add('A', ['B', 'C']) + graph.add('B', ['D']) + graph.add('C', ['D']) // Shared dependency + graph.add('D', []) + + expect(graph.delete('D')).toEqual(new Set(['A', 'B', 'C', 'D'])) + }) + + it('merges dependencies correctly', () => { + // Add module A with dependency B + graph.add('A', ['B']) + // Merge new dependency C into module A (B should remain) + graph.add('A', ['C']) + + // Deleting B should remove A as well, since A depends on B. + expect(graph.delete('B')).toEqual(new Set(['B', 'A'])) + }) + + it('handles cycles gracefully', () => { + // Create a cycle: A -> B, B -> C, C -> A. + graph.add('A', ['B']) + graph.add('B', ['C']) + graph.add('C', ['A']) + + // Deleting any module in the cycle should delete all modules in the cycle. + expect(graph.delete('A')).toEqual(new Set(['A', 'B', 'C'])) + }) + + it('cleans up dependencies when deletion', () => { + // Setup A -> B relationship. + graph.add('A', ['B']) + graph.add('B', []) + + // Deleting B should remove both B and A from the graph. + expect(graph.delete('B')).toEqual(new Set(['B', 'A'])) + + // After deletion, add modules again. + graph.add('C', []) + graph.add('A', ['C']) // Now A depends only on C. + + expect(graph.delete('C')).toEqual(new Set(['C', 'A'])) + }) + + it('handles independent modules', () => { + // Modules with no dependencies. + graph.add('X', []) + graph.add('Y', []) + + // Deletion of one should only remove that module. + expect(graph.delete('X')).toEqual(new Set(['X'])) + expect(graph.delete('Y')).toEqual(new Set(['Y'])) + }) +}) diff --git a/src/node/plugins/dynamicRoutesPlugin.ts b/src/node/plugins/dynamicRoutesPlugin.ts index 2fa647a8..d424a89b 100644 --- a/src/node/plugins/dynamicRoutesPlugin.ts +++ b/src/node/plugins/dynamicRoutesPlugin.ts @@ -1,6 +1,7 @@ import fs from 'fs-extra' import path from 'node:path' import c from 'picocolors' +import { isMatch } from 'picomatch' import { glob } from 'tinyglobby' import { loadConfigFromFile, @@ -9,12 +10,48 @@ import { type Logger, type Plugin } from 'vite' +import type { Awaitable } from '../shared' import { type SiteConfig, type UserConfig } from '../siteConfig' +import { ModuleGraph } from '../utils/moduleGraph' import { resolveRewrites } from './rewritesPlugin' -export const dynamicRouteRE = /\[(\w+?)\]/g +interface UserRouteConfig { + params: Record + content?: string +} + +export type ResolvedRouteConfig = UserRouteConfig & { + /** + * the raw route (relative to src root), e.g. foo/[bar].md + */ + route: string + /** + * the actual path with params resolved (relative to src root), e.g. foo/1.md + */ + path: string + /** + * absolute fs path + */ + fullPath: string +} + +export interface RouteModule { + watch?: string[] | string + paths: + | UserRouteConfig[] + | ((watchedFiles: string[]) => Awaitable) +} + +interface ResolvedRouteModule { + watch: string[] | undefined + routes: ResolvedRouteConfig[] | undefined + loader: RouteModule['paths'] +} -let fileToModulesMap: Record> = {} +const dynamicRouteRE = /\[(\w+?)\]/g + +const routeModuleCache = new Map() +let moduleGraph = new ModuleGraph() export async function resolvePages( srcDir: string, @@ -47,58 +84,25 @@ export async function resolvePages( ;(dynamicRouteRE.test(file) ? dynamicRouteFiles : pages).push(file) }) - const { routes, fileToModulesMap: fileToModulesMap_ } = - await resolveDynamicRoutes(srcDir, dynamicRouteFiles, logger) + const dynamicRoutes = await resolveDynamicRoutes( + srcDir, + dynamicRouteFiles, + logger + ) - pages.push(...routes.map((r) => r.path)) - fileToModulesMap = fileToModulesMap_ + pages.push(...dynamicRoutes.map((r) => r.path)) const rewrites = resolveRewrites(pages, userConfig.rewrites) return { pages, - dynamicRoutes: { routes }, + dynamicRoutes, rewrites, // @ts-expect-error internal flag to reload resolution cache in ../markdownToVue.ts __dirty: true } } -interface UserRouteConfig { - params: Record - content?: string -} - -interface RouteModule { - path: string - config: { - watch?: string[] | string - paths: - | UserRouteConfig[] - | (( - watchedFiles: string[] - ) => UserRouteConfig[] | Promise) - } - dependencies: string[] -} - -const routeModuleCache = new Map() - -export type ResolvedRouteConfig = UserRouteConfig & { - /** - * the raw route (relative to src root), e.g. foo/[bar].md - */ - route: string - /** - * the actual path with params resolved (relative to src root), e.g. foo/1.md - */ - path: string - /** - * absolute fs path - */ - fullPath: string -} - export const dynamicRoutesPlugin = async ( config: SiteConfig ): Promise => { @@ -110,7 +114,7 @@ export const dynamicRoutesPlugin = async ( const normalizedId = id.startsWith(config.srcDir) ? id : normalizePath(path.resolve(config.srcDir, id.replace(/^\//, ''))) - const matched = config.dynamicRoutes.routes.find( + const matched = config.dynamicRoutes.find( (r) => r.fullPath === normalizedId ) if (matched) { @@ -119,11 +123,11 @@ export const dynamicRoutesPlugin = async ( }, load(id) { - const matched = config.dynamicRoutes.routes.find((r) => r.fullPath === id) + const matched = config.dynamicRoutes.find((r) => r.fullPath === id) if (matched) { const { route, params, content } = matched const routeFile = normalizePath(path.resolve(config.srcDir, route)) - fileToModulesMap[routeFile].add(id) + moduleGraph.add(id, [routeFile]) let baseContent = fs.readFileSync(routeFile, 'utf-8') @@ -147,51 +151,49 @@ export const dynamicRoutesPlugin = async ( async hotUpdate({ file, modules: existingMods }) { if (this.environment.name !== 'client') return + const modules = new Set() const normalizedFile = normalizePath(file) - // Invalidate any cached route modules whose key or dependencies include the changed file. - for (const [cacheKey, mod] of routeModuleCache.entries()) { - const normalizedCacheKey = normalizePath(cacheKey) - if ( - normalizedCacheKey === normalizedFile || - mod.dependencies.some( - (dep) => normalizePath(path.resolve(dep)) === normalizedFile - ) - ) { - routeModuleCache.delete(cacheKey) + + // Trigger update if a module or its dependencies changed. + for (const id of moduleGraph.delete(normalizedFile)) { + routeModuleCache.delete(id) + const mod = this.environment.moduleGraph.getModuleById(id) + if (mod) { + modules.add(mod) } } - const modules: EnvironmentModuleNode[] = [] + // Also check if the file matches any custom watch patterns. + let watchedFileChanged = false + for (const [, route] of routeModuleCache) { + if (route.watch && isMatch(normalizedFile, route.watch)) { + route.routes = undefined + watchedFileChanged = true + } + } - const mods = fileToModulesMap[normalizedFile] - if (mods) { + if (modules.size || watchedFileChanged) { // path loader module or deps updated, reset loaded routes - if (!normalizedFile.endsWith('.md')) { + if (!normalizedFile.endsWith('.md') || watchedFileChanged) { Object.assign( config, await resolvePages(config.srcDir, config.userConfig, config.logger) ) } - for (const id of mods) { - const mod = this.environment.moduleGraph.getModuleById(id) - if (mod) { - modules.push(mod) - } - } } - return modules.length > 0 ? [...existingMods, ...modules] : undefined + return modules.size ? [...existingMods, ...modules] : undefined } } } -export async function resolveDynamicRoutes( +async function resolveDynamicRoutes( srcDir: string, routes: string[], logger: Logger -) { +): Promise { const pendingResolveRoutes: Promise[] = [] - const routeFileToModulesMap: Record> = {} + const newModuleGraph = moduleGraph.clone() for (const route of routes) { // locate corresponding route paths file @@ -214,78 +216,80 @@ export async function resolveDynamicRoutes( } // load the paths loader module - let mod = routeModuleCache.get(pathsFile) - if (!mod) { + let watch: ResolvedRouteModule['watch'] + let loader: ResolvedRouteModule['loader'] + + const existing = routeModuleCache.get(normalizePath(pathsFile)) + if (existing) { + // use cached routes if not invalidated by hmr + if (existing.routes) { + pendingResolveRoutes.push(Promise.resolve(existing.routes)) + continue + } + + ;({ watch, loader } = existing) + } else { + let mod try { - mod = (await loadConfigFromFile( + mod = await loadConfigFromFile( {} as any, pathsFile, undefined, 'silent' - )) as RouteModule - routeModuleCache.set(pathsFile, mod) + ) } catch (err: any) { logger.warn( `${c.yellow(`Failed to load ${pathsFile}:`)}\n${err.message}\n${err.stack}` ) continue } - } - const loader = mod.config.paths - if (!loader) { - logger.warn( - c.yellow( - `Invalid paths file export in ${pathsFile}. ` + - `Missing "paths" property from default export.` + if (!mod) { + logger.warn( + c.yellow( + `Invalid paths file export in ${pathsFile}. ` + + `Missing "default" export.` + ) ) - ) - continue - } + continue + } - // Create or retrieve the set of virtual module IDs affected by this route. - const routeKey = normalizePath(path.resolve(srcDir, route)) - const matchedModuleIds = - routeFileToModulesMap[routeKey] || new Set() - routeFileToModulesMap[routeKey] = matchedModuleIds - - // Track loader dependencies (merging sets if shared) - for (const dep of mod.dependencies) { - const depPath = normalizePath(path.resolve(dep)) - if (!routeFileToModulesMap[depPath]) { - routeFileToModulesMap[depPath] = matchedModuleIds - } else { - for (const id of matchedModuleIds) { - routeFileToModulesMap[depPath].add(id) - } + const routeModule = mod.config as RouteModule + + loader = routeModule.paths + if (!loader) { + logger.warn( + c.yellow( + `Invalid paths file export in ${pathsFile}. ` + + `Missing "paths" property from default export.` + ) + ) + continue } - } - // Process custom watch files if provided. - let watch: string[] | undefined - if (mod.config.watch) { watch = - typeof mod.config.watch === 'string' - ? [mod.config.watch] - : mod.config.watch - watch = watch.map((p) => - p.startsWith('.') - ? normalizePath(path.resolve(path.dirname(pathsFile), p)) - : normalizePath(p) - ) - for (const watchFile of watch) { - if (!routeFileToModulesMap[watchFile]) { - routeFileToModulesMap[watchFile] = matchedModuleIds - } else { - for (const id of matchedModuleIds) { - routeFileToModulesMap[watchFile].add(id) - } - } + typeof routeModule.watch === 'string' + ? [routeModule.watch] + : routeModule.watch + if (watch) { + watch = watch.map((p) => + p.startsWith('.') + ? normalizePath(path.resolve(path.dirname(pathsFile), p)) + : normalizePath(p) + ) } + + // record deps for hmr + newModuleGraph.add( + normalizePath(pathsFile), + mod.dependencies.map((f) => normalizePath(path.resolve(f))) + ) + newModuleGraph.add(fullPath, [normalizePath(pathsFile)]) } const resolveRoute = async (): Promise => { let pathsData: UserRouteConfig[] + if (typeof loader === 'function') { let watchedFiles: string[] = [] if (watch) { @@ -300,7 +304,8 @@ export async function resolveDynamicRoutes( } else { pathsData = loader } - return pathsData.map((userConfig) => { + + const routes = pathsData.map((userConfig) => { const resolvedPath = route.replace( dynamicRouteRE, (_, key) => userConfig.params[key] @@ -312,12 +317,17 @@ export async function resolveDynamicRoutes( ...userConfig } }) + + routeModuleCache.set(normalizePath(pathsFile), { watch, routes, loader }) + + return routes } + pendingResolveRoutes.push(resolveRoute()) } - return { - routes: (await Promise.all(pendingResolveRoutes)).flat(), - fileToModulesMap: routeFileToModulesMap - } + const resolvedRoutes = (await Promise.all(pendingResolveRoutes)).flat() + moduleGraph = newModuleGraph + + return resolvedRoutes } diff --git a/src/node/utils/moduleGraph.ts b/src/node/utils/moduleGraph.ts new file mode 100644 index 00000000..ff521014 --- /dev/null +++ b/src/node/utils/moduleGraph.ts @@ -0,0 +1,115 @@ +export class ModuleGraph { + // Each module is tracked with its dependencies and dependents. + private nodes: Map< + string, + { dependencies: Set; dependents: Set } + > = new Map() + + /** + * Adds or updates a module by merging the provided dependencies + * with any existing ones. + * + * For every new dependency, the module is added to that dependency's + * 'dependents' set. + * + * @param module - The module to add or update. + * @param dependencies - Array of module names that the module depends on. + */ + add(module: string, dependencies: string[]): void { + // Ensure the module exists in the graph. + if (!this.nodes.has(module)) { + this.nodes.set(module, { + dependencies: new Set(), + dependents: new Set() + }) + } + const moduleNode = this.nodes.get(module)! + + // Merge the new dependencies with any that already exist. + for (const dep of dependencies) { + if (!moduleNode.dependencies.has(dep)) { + moduleNode.dependencies.add(dep) + // Ensure the dependency exists in the graph. + if (!this.nodes.has(dep)) { + this.nodes.set(dep, { + dependencies: new Set(), + dependents: new Set() + }) + } + // Add the module as a dependent of the dependency. + this.nodes.get(dep)!.dependents.add(module) + } + } + } + + /** + * Deletes a module and all modules that (transitively) depend on it. + * + * This method performs a depth-first search from the target module, + * collects all affected modules, and then removes them from the graph, + * cleaning up their references from other nodes. + * + * @param module - The module to delete. + * @returns A Set containing the deleted module and all modules that depend on it. + */ + delete(module: string): Set { + const deleted = new Set() + const stack: string[] = [module] + + // Traverse the reverse dependency graph (using dependents). + while (stack.length) { + const current = stack.pop()! + if (!deleted.has(current)) { + deleted.add(current) + const node = this.nodes.get(current) + if (node) { + for (const dependent of node.dependents) { + stack.push(dependent) + } + } + } + } + + // Remove deleted nodes from the graph. + // For each deleted node, also remove it from its dependencies' dependents. + for (const mod of deleted) { + const node = this.nodes.get(mod) + if (node) { + for (const dep of node.dependencies) { + const depNode = this.nodes.get(dep) + if (depNode) { + depNode.dependents.delete(mod) + } + } + } + this.nodes.delete(mod) + } + + return deleted + } + + /** + * Clears all modules from the graph. + */ + clear(): void { + this.nodes.clear() + } + + /** + * Creates a deep clone of the ModuleGraph instance. + * This is useful for preserving the state of the graph + * before making modifications. + * + * @returns A new ModuleGraph instance with the same state as the original. + */ + clone(): ModuleGraph { + const clone = new ModuleGraph() + for (const [module, { dependencies, dependents }] of this.nodes) { + clone.nodes.set(module, { + dependencies: new Set(dependencies), + dependents: new Set(dependents) + }) + } + return clone + } +}