Skip to content

Commit 366919e

Browse files
committed
Fix transient .quarto_ipynb files accumulating during preview (#14281)
Preview re-renders delete the fileInformationCache entry to pick up file changes, but this loses track of the transient .quarto_ipynb path. The collision-avoidance loop in jupyter.ts target() then sees the old file on disk and creates numbered variants (_1, _2, etc.) that are never cleaned up until preview exits. Add invalidateForFile() to FileInformationCacheMap that cleans up any transient notebook file from disk before removing the cache entry. Replace the bare delete() call in renderForPreview() with this method. Fixes #14281
1 parent 8dd9762 commit 366919e

4 files changed

Lines changed: 107 additions & 5 deletions

File tree

src/command/preview/preview.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,9 +427,10 @@ export async function renderForPreview(
427427
// Invalidate file cache for the file being rendered so changes are picked up.
428428
// The project context persists across re-renders in preview mode, but the
429429
// fileInformationCache contains file content that needs to be refreshed.
430-
// TODO(#13955): Consider adding a dedicated invalidateForFile() method on ProjectContext
430+
// Uses invalidateForFile() to also clean up transient notebook files
431+
// (.quarto_ipynb) from disk before removing the cache entry (#14281).
431432
if (project?.fileInformationCache) {
432-
project.fileInformationCache.delete(file);
433+
project.fileInformationCache.invalidateForFile(file);
433434
}
434435

435436
// render

src/project/project-shared.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { readAndValidateYamlFromFile } from "../core/schema/validated-yaml.ts";
2222
import {
2323
FileInclusion,
2424
FileInformation,
25+
FileInformationCache,
2526
kProjectOutputDir,
2627
kProjectType,
2728
ProjectConfig,
@@ -660,7 +661,7 @@ export async function projectResolveBrand(
660661
// Implements Cloneable but shares state intentionally - in preview mode,
661662
// the project context is reused across renders and cache state must persist.
662663
export class FileInformationCacheMap extends Map<string, FileInformation>
663-
implements Cloneable<Map<string, FileInformation>> {
664+
implements FileInformationCache, Cloneable<Map<string, FileInformation>> {
664665
override get(key: string): FileInformation | undefined {
665666
return super.get(normalizePath(key));
666667
}
@@ -681,6 +682,22 @@ export class FileInformationCacheMap extends Map<string, FileInformation>
681682
// return normalized keys as stored. Code iterating over the cache sees
682683
// normalized paths, which is consistent with how keys are stored.
683684

685+
// Removes a cache entry and cleans up any associated transient files.
686+
// In preview mode, this should be used instead of delete() to ensure
687+
// transient notebooks (.quarto_ipynb) are removed from disk before the
688+
// cache entry is dropped. Without this, the collision-avoidance logic
689+
// in jupyter.ts target() creates numbered variants on each re-render.
690+
invalidateForFile(key: string): void {
691+
const existing = this.get(key);
692+
if (existing?.target?.data) {
693+
const data = existing.target.data as { transient?: boolean };
694+
if (data.transient && existing.target.input) {
695+
safeRemoveSync(existing.target.input);
696+
}
697+
}
698+
this.delete(key);
699+
}
700+
684701
// Returns this instance (shared reference) rather than a copy.
685702
// This is intentional: in preview mode, project context is cloned for
686703
// each render but the cache must be shared so invalidations persist.

src/project/types.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ export type FileInformation = {
6868
brand?: LightDarkBrandDarkFlag;
6969
};
7070

71+
export interface FileInformationCache extends Map<string, FileInformation> {
72+
// Removes a cache entry and cleans up any associated transient files from disk.
73+
// Use this instead of delete() when invalidating entries that may reference
74+
// transient notebooks (.quarto_ipynb) to prevent file accumulation.
75+
invalidateForFile(key: string): void;
76+
}
77+
7178
export interface ProjectContext extends Cloneable<ProjectContext> {
7279
dir: string;
7380
engines: string[];
@@ -76,7 +83,7 @@ export interface ProjectContext extends Cloneable<ProjectContext> {
7683
notebookContext: NotebookContext;
7784
outputNameIndex?: Map<string, { file: string; format: Format } | undefined>;
7885

79-
fileInformationCache: Map<string, FileInformation>;
86+
fileInformationCache: FileInformationCache;
8087

8188
// This is a cache of _brand.yml for a project
8289
brandCache?: { brand?: LightDarkBrandDarkFlag };
@@ -182,7 +189,7 @@ export interface EngineProjectContext {
182189
* For file information cache management
183190
* Used for the transient notebook tracking in Jupyter
184191
*/
185-
fileInformationCache: Map<string, FileInformation>;
192+
fileInformationCache: FileInformationCache;
186193

187194
/**
188195
* Get the output directory for the project

tests/unit/project/file-information-cache.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
import { unitTest } from "../../test.ts";
1111
import { assert } from "testing/asserts";
12+
import { asMappedString } from "../../../src/core/lib/mapped-text.ts";
13+
import { existsSync } from "../../../src/deno_ral/fs.ts";
1214
import { join, relative } from "../../../src/deno_ral/path.ts";
1315
import {
1416
ensureFileInformationCache,
@@ -130,3 +132,78 @@ unitTest(
130132
);
131133
},
132134
);
135+
136+
// deno-lint-ignore require-await
137+
unitTest(
138+
"fileInformationCache - invalidateForFile deletes transient notebook file",
139+
async () => {
140+
const project = createMockProjectContext();
141+
const sourcePath = join(project.dir, "doc.qmd");
142+
143+
// Create a real temp file simulating a transient .quarto_ipynb
144+
const notebookPath = join(project.dir, "doc.quarto_ipynb");
145+
Deno.writeTextFileSync(notebookPath, '{"cells": []}');
146+
assert(existsSync(notebookPath), "Temp notebook file should exist");
147+
148+
// Populate cache entry with a transient target pointing to the file
149+
const entry = ensureFileInformationCache(project, sourcePath);
150+
entry.target = {
151+
source: sourcePath,
152+
input: notebookPath,
153+
markdown: asMappedString(""),
154+
metadata: {},
155+
data: { transient: true, kernelspec: {} },
156+
};
157+
158+
// Invalidate the cache entry for this file
159+
project.fileInformationCache.invalidateForFile(sourcePath);
160+
161+
// The transient file should be deleted from disk
162+
assert(
163+
!existsSync(notebookPath),
164+
"Transient notebook file should be deleted on invalidation",
165+
);
166+
// The cache entry should be removed
167+
assert(
168+
!project.fileInformationCache.has(sourcePath),
169+
"Cache entry should be removed after invalidation",
170+
);
171+
},
172+
);
173+
174+
// deno-lint-ignore require-await
175+
unitTest(
176+
"fileInformationCache - invalidateForFile preserves non-transient files",
177+
async () => {
178+
const project = createMockProjectContext();
179+
const sourcePath = join(project.dir, "notebook.ipynb");
180+
181+
// Create a real file simulating a user's .ipynb (non-transient)
182+
const notebookPath = join(project.dir, "notebook.ipynb");
183+
Deno.writeTextFileSync(notebookPath, '{"cells": []}');
184+
185+
// Populate cache entry with a non-transient target
186+
const entry = ensureFileInformationCache(project, sourcePath);
187+
entry.target = {
188+
source: sourcePath,
189+
input: notebookPath,
190+
markdown: asMappedString(""),
191+
metadata: {},
192+
data: { transient: false, kernelspec: {} },
193+
};
194+
195+
// Invalidate the cache entry
196+
project.fileInformationCache.invalidateForFile(sourcePath);
197+
198+
// The non-transient file should NOT be deleted
199+
assert(
200+
existsSync(notebookPath),
201+
"Non-transient file should be preserved on invalidation",
202+
);
203+
// But the cache entry should still be removed
204+
assert(
205+
!project.fileInformationCache.has(sourcePath),
206+
"Cache entry should be removed after invalidation",
207+
);
208+
},
209+
);

0 commit comments

Comments
 (0)