From a6417c447af8f0ff203ffb8730e53e4a7dfafb23 Mon Sep 17 00:00:00 2001 From: kabirgh <15871468+kabirgh@users.noreply.github.com> Date: Sat, 24 Feb 2024 02:40:42 +0000 Subject: [PATCH] fix(fast rebuild): handle added an deleted markdown correctly (#921) * Handle added files correctly * Handle deletes properly * addGraph renamed to mergeGraph --- quartz/build.ts | 33 +++++++++++++++++++++------------ quartz/depgraph.test.ts | 22 ++++++++++++++++++++++ quartz/depgraph.ts | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/quartz/build.ts b/quartz/build.ts index 3d95f315f..d72b8ddf4 100644 --- a/quartz/build.ts +++ b/quartz/build.ts @@ -185,9 +185,14 @@ async function partialRebuildFromEntrypoint( const emitterGraph = (await emitter.getDependencyGraph?.(ctx, processedFiles, staticResources)) ?? null - // emmiter may not define a dependency graph. nothing to update if so if (emitterGraph) { - dependencies[emitter.name]?.updateIncomingEdgesForNode(emitterGraph, fp) + const existingGraph = dependencies[emitter.name] + if (existingGraph !== null) { + existingGraph.mergeGraph(emitterGraph) + } else { + // might be the first time we're adding a mardown file + dependencies[emitter.name] = emitterGraph + } } } break @@ -224,7 +229,6 @@ async function partialRebuildFromEntrypoint( // EMIT perf.addEvent("rebuild") let emittedFiles = 0 - const destinationsToDelete = new Set() for (const emitter of cfg.plugins.emitters) { const depGraph = dependencies[emitter.name] @@ -264,11 +268,6 @@ async function partialRebuildFromEntrypoint( // and supply [a.md, b.md] to the emitter const upstreams = [...depGraph.getLeafNodeAncestors(fp)] as FilePath[] - if (action === "delete" && upstreams.length === 1) { - // if there's only one upstream, the destination is solely dependent on this file - destinationsToDelete.add(upstreams[0]) - } - const upstreamContent = upstreams // filter out non-markdown files .filter((file) => contentMap.has(file)) @@ -291,14 +290,24 @@ async function partialRebuildFromEntrypoint( console.log(`Emitted ${emittedFiles} files to \`${argv.output}\` in ${perf.timeSince("rebuild")}`) // CLEANUP - // delete files that are solely dependent on this file - await rimraf([...destinationsToDelete]) + const destinationsToDelete = new Set() for (const file of toRemove) { // remove from cache contentMap.delete(file) - // remove the node from dependency graphs - Object.values(dependencies).forEach((depGraph) => depGraph?.removeNode(file)) + Object.values(dependencies).forEach((depGraph) => { + // remove the node from dependency graphs + depGraph?.removeNode(file) + // remove any orphan nodes. eg if a.md is deleted, a.html is orphaned and should be removed + const orphanNodes = depGraph?.removeOrphanNodes() + orphanNodes?.forEach((node) => { + // only delete files that are in the output directory + if (node.startsWith(argv.output)) { + destinationsToDelete.add(node) + } + }) + }) } + await rimraf([...destinationsToDelete]) toRemove.clear() release() diff --git a/quartz/depgraph.test.ts b/quartz/depgraph.test.ts index 43eb4024f..062f13e35 100644 --- a/quartz/depgraph.test.ts +++ b/quartz/depgraph.test.ts @@ -39,6 +39,28 @@ describe("DepGraph", () => { }) }) + describe("mergeGraph", () => { + test("merges two graphs", () => { + const graph = new DepGraph() + graph.addEdge("A.md", "A.html") + + const other = new DepGraph() + other.addEdge("B.md", "B.html") + + graph.mergeGraph(other) + + const expected = { + nodes: ["A.md", "A.html", "B.md", "B.html"], + edges: [ + ["A.md", "A.html"], + ["B.md", "B.html"], + ], + } + + assert.deepStrictEqual(graph.export(), expected) + }) + }) + describe("updateIncomingEdgesForNode", () => { test("merges when node exists", () => { // A.md -> B.md -> B.html diff --git a/quartz/depgraph.ts b/quartz/depgraph.ts index 1efad0770..3d048cd83 100644 --- a/quartz/depgraph.ts +++ b/quartz/depgraph.ts @@ -39,12 +39,26 @@ export default class DepGraph { } } + // Remove node and all edges connected to it removeNode(node: T): void { if (this._graph.has(node)) { + // first remove all edges so other nodes don't have references to this node + for (const target of this._graph.get(node)!.outgoing) { + this.removeEdge(node, target) + } + for (const source of this._graph.get(node)!.incoming) { + this.removeEdge(source, node) + } this._graph.delete(node) } } + forEachNode(callback: (node: T) => void): void { + for (const node of this._graph.keys()) { + callback(node) + } + } + hasEdge(from: T, to: T): boolean { return Boolean(this._graph.get(from)?.outgoing.has(to)) } @@ -92,6 +106,15 @@ export default class DepGraph { // DEPENDENCY ALGORITHMS + // Add all nodes and edges from other graph to this graph + mergeGraph(other: DepGraph): void { + other.forEachEdge(([source, target]) => { + this.addNode(source) + this.addNode(target) + this.addEdge(source, target) + }) + } + // For the node provided: // If node does not exist, add it // If an incoming edge was added in other, it is added in this graph @@ -112,6 +135,24 @@ export default class DepGraph { }) } + // Remove all nodes that do not have any incoming or outgoing edges + // A node may be orphaned if the only node pointing to it was removed + removeOrphanNodes(): Set { + let orphanNodes = new Set() + + this.forEachNode((node) => { + if (this.inDegree(node) === 0 && this.outDegree(node) === 0) { + orphanNodes.add(node) + } + }) + + orphanNodes.forEach((node) => { + this.removeNode(node) + }) + + return orphanNodes + } + // Get all leaf nodes (i.e. destination paths) reachable from the node provided // Eg. if the graph is A -> B -> C // D ---^