From a1946d5f1ba580f5402b438b62a39a878ea3f275 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 15 Mar 2024 14:56:35 -0700 Subject: [PATCH] normalize file path; deterministic file resolution --- src/dataloader.ts | 41 +++++++++++-------- src/javascript/transpile.ts | 13 +++++- src/render.ts | 10 ++++- src/resolvers.ts | 2 + test/build-test.ts | 2 +- test/dataloaders-test.ts | 22 ++++++---- test/javascript/transpile-test.ts | 4 +- test/output/build/archives.posix/tar.html | 20 ++++----- test/output/build/archives.posix/zip.html | 12 +++--- test/output/build/files/files.html | 8 ++-- .../build/files/subsection/subfiles.html | 2 +- test/output/build/imports/foo/foo.html | 2 +- test/output/build/missing-file/index.html | 2 +- test/output/build/multi/index.html | 2 +- test/output/build/simple/simple.html | 2 +- test/output/template-file-attachment.js | 2 +- 16 files changed, 89 insertions(+), 57 deletions(-) diff --git a/src/dataloader.ts b/src/dataloader.ts index 4ff211279..e427240b5 100644 --- a/src/dataloader.ts +++ b/src/dataloader.ts @@ -130,33 +130,42 @@ export class LoaderResolver { } /** - * Get the actual path of a file. For data loaders, it is the output if - * already available (cached). In build this is always the case (unless the - * corresponding data loader fails). However in preview we return the page - * before running the data loaders (which will run on demand from the page), - * so there might be a temporary discrepancy when a cache is stale. + * Returns the path to the backing file during preview, which is the source + * file for the associated data loader if the file is generated by a loader. */ - private getFilePath(name: string): string { + private getSourceFilePath(name: string): string { let path = name; if (!existsSync(join(this.root, path))) { const loader = this.find(path); - if (loader) { - path = relative(this.root, loader.path); - if (name !== path) { - const cachePath = join(".observablehq", "cache", name); - if (existsSync(join(this.root, cachePath))) path = cachePath; - } - } + if (loader) path = relative(this.root, loader.path); + } + return path; + } + + /** + * Returns the path to the backing file during build, which is the cached + * output file if the file is generated by a loader. + */ + private getOutputFilePath(name: string): string { + let path = name; + if (!existsSync(join(this.root, path))) { + const loader = this.find(path); + if (loader) path = join(".observablehq", "cache", name); } return path; } getFileHash(name: string): string { - return getFileHash(this.root, this.getFilePath(name)); + return getFileHash(this.root, this.getSourceFilePath(name)); + } + + getSourceLastModified(name: string): number | undefined { + const entry = getFileInfo(this.root, this.getSourceFilePath(name)); + return entry && Math.floor(entry.mtimeMs); } - getLastModified(name: string): number | undefined { - const entry = getFileInfo(this.root, this.getFilePath(name)); + getOutputLastModified(name: string): number | undefined { + const entry = getFileInfo(this.root, this.getOutputFilePath(name)); return entry && Math.floor(entry.mtimeMs); } diff --git a/src/javascript/transpile.ts b/src/javascript/transpile.ts index ff1ffe05a..311eededa 100644 --- a/src/javascript/transpile.ts +++ b/src/javascript/transpile.ts @@ -5,6 +5,7 @@ import {simple} from "acorn-walk"; import {isPathImport, relativePath, resolvePath} from "../path.js"; import {getModuleResolver} from "../resolvers.js"; import {Sourcemap} from "../sourcemap.js"; +import type {FileExpression} from "./files.js"; import {findFiles} from "./files.js"; import type {ExportNode, ImportNode} from "./imports.js"; import {hasImportDeclaration, isImportMetaResolve} from "./imports.js"; @@ -15,10 +16,11 @@ import {getStringLiteralValue, isStringLiteral} from "./source.js"; export interface TranspileOptions { id: string; + path: string; resolveImport?: (specifier: string) => string; } -export function transpileJavaScript(node: JavaScriptNode, {id, resolveImport}: TranspileOptions): string { +export function transpileJavaScript(node: JavaScriptNode, {id, path, resolveImport}: TranspileOptions): string { let async = node.async; const inputs = Array.from(new Set(node.references.map((r) => r.name))); const outputs = Array.from(new Set(node.declarations?.map((r) => r.name))); @@ -28,6 +30,7 @@ export function transpileJavaScript(node: JavaScriptNode, {id, resolveImport}: T const output = new Sourcemap(node.input).trim(); rewriteImportDeclarations(output, node.body, resolveImport); rewriteImportExpressions(output, node.body, resolveImport); + rewriteFileExpressions(output, node.files, path); if (display) output.insertLeft(0, "display(await(\n").insertRight(node.input.length, "\n))"); output.insertLeft(0, `, body: ${async ? "async " : ""}(${inputs}) => {\n`); if (outputs.length) output.insertLeft(0, `, outputs: ${JSON.stringify(outputs)}`); @@ -100,6 +103,14 @@ export async function transpileModule( return String(output); } +function rewriteFileExpressions(output: Sourcemap, files: FileExpression[], path: string): void { + for (const {name, node} of files) { + const source = node.arguments[0]; + const resolved = relativePath(path, resolvePath(path, name)); + output.replaceLeft(source.start, source.end, JSON.stringify(resolved)); + } +} + function rewriteImportExpressions( output: Sourcemap, body: Node, diff --git a/src/render.ts b/src/render.ts index 8672a8ef1..9aefbc914 100644 --- a/src/render.ts +++ b/src/render.ts @@ -65,7 +65,13 @@ import ${preview || page.code.length ? `{${preview ? "open, " : ""}define} from : "" }${data?.sql ? `\nimport {registerTable} from ${JSON.stringify(resolveImport("npm:@observablehq/duckdb"))};` : ""}${ files.size - ? `\n${renderFiles(files, resolveFile, (name: string) => loaders.getLastModified(resolvePath(path, name)))}` + ? `\n${renderFiles( + files, + resolveFile, + preview + ? (name: string) => loaders.getSourceLastModified(resolvePath(path, name)) + : (name: string) => loaders.getOutputLastModified(resolvePath(path, name)) + )}` : "" }${ data?.sql @@ -75,7 +81,7 @@ import ${preview || page.code.length ? `{${preview ? "open, " : ""}define} from : "" } ${preview ? `\nopen({hash: ${JSON.stringify(resolvers.hash)}, eval: (body) => eval(body)});\n` : ""}${page.code - .map(({node, id}) => `\n${transpileJavaScript(node, {id, resolveImport})}`) + .map(({node, id}) => `\n${transpileJavaScript(node, {id, path, resolveImport})}`) .join("")}`)} ${sidebar ? html`\n${await renderSidebar(title, pages, root, path, search, normalizeLink)}` : ""}${ toc.show ? html`\n${renderToc(findHeaders(page), toc.label)}` : "" diff --git a/src/resolvers.ts b/src/resolvers.ts index 3e231ef63..cc817a94c 100644 --- a/src/resolvers.ts +++ b/src/resolvers.ts @@ -12,6 +12,7 @@ import {extractNpmSpecifier, populateNpmCache, resolveNpmImport, resolveNpmImpor import {isAssetPath, isPathImport, relativePath, resolveLocalPath, resolvePath} from "./path.js"; export interface Resolvers { + path: string; hash: string; assets: Set; // like files, but not registered for FileAttachment files: Set; @@ -259,6 +260,7 @@ export async function getResolvers( } return { + path, hash: hash.digest("hex"), assets, files, diff --git a/test/build-test.ts b/test/build-test.ts index e6f68c5f0..3ad3bf700 100644 --- a/test/build-test.ts +++ b/test/build-test.ts @@ -26,7 +26,7 @@ describe("build", () => { if (isEmpty(path)) continue; const only = name.startsWith("only."); const skip = name.startsWith("skip."); - const outname = only || skip ? name.slice(5) : name; + const outname = name.replace(/^only\.|skip\./, ""); (only ? it.only : skip || diff --git a/test/dataloaders-test.ts b/test/dataloaders-test.ts index af2b13855..24970cee0 100644 --- a/test/dataloaders-test.ts +++ b/test/dataloaders-test.ts @@ -114,23 +114,26 @@ describe("LoaderResolver.getFileHash(path)", () => { }); }); -describe("LoaderResolver.getLastModified(path)", () => { +describe("LoaderResolver.get{Source,Output}LastModified(path)", () => { const time1 = new Date(Date.UTC(2023, 11, 1)); const time2 = new Date(Date.UTC(2024, 2, 1)); const loaders = new LoaderResolver({root: "test"}); - it("returns the last modification time for a simple file", async () => { + it("both return the last modification time for a simple file", async () => { await utimes("test/input/loader/simple.txt", time1, time1); - assert.strictEqual(loaders.getLastModified("input/loader/simple.txt"), +time1); + assert.strictEqual(loaders.getSourceLastModified("input/loader/simple.txt"), +time1); + assert.strictEqual(loaders.getOutputLastModified("input/loader/simple.txt"), +time1); }); - it("returns an undefined last modification time for a missing file", async () => { - assert.strictEqual(loaders.getLastModified("input/loader/missing.txt"), undefined); + it("both return an undefined last modification time for a missing file", async () => { + assert.strictEqual(loaders.getSourceLastModified("input/loader/missing.txt"), undefined); + assert.strictEqual(loaders.getOutputLastModified("input/loader/missing.txt"), undefined); }); - it("returns the last modification time for a cached data loader", async () => { + it("returns the last modification time of the loader in preview, and of the cache, on build", async () => { await utimes("test/input/loader/cached.txt.sh", time1, time1); await mkdir("test/.observablehq/cache/input/loader/", {recursive: true}); await writeFile("test/.observablehq/cache/input/loader/cached.txt", "2024-03-01 00:00:00"); await utimes("test/.observablehq/cache/input/loader/cached.txt", time2, time2); - assert.strictEqual(loaders.getLastModified("input/loader/cached.txt"), +time2); + assert.strictEqual(loaders.getSourceLastModified("input/loader/cached.txt"), +time1); + assert.strictEqual(loaders.getOutputLastModified("input/loader/cached.txt"), +time2); // clean up try { await unlink("test/.observablehq/cache/input/loader/cached.txt"); @@ -139,8 +142,9 @@ describe("LoaderResolver.getLastModified(path)", () => { // ignore; } }); - it("returns the last modification time for a data loader that has no cache", async () => { + it("returns the last modification time of the data loader in preview, and null in build, when there is no cache", async () => { await utimes("test/input/loader/not-cached.txt.sh", time1, time1); - assert.strictEqual(loaders.getLastModified("input/loader/not-cached.txt"), +time1); + assert.strictEqual(loaders.getSourceLastModified("input/loader/not-cached.txt"), +time1); + assert.strictEqual(loaders.getOutputLastModified("input/loader/not-cached.txt"), undefined); }); }); diff --git a/test/javascript/transpile-test.ts b/test/javascript/transpile-test.ts index b1e21aa56..c898d11c8 100644 --- a/test/javascript/transpile-test.ts +++ b/test/javascript/transpile-test.ts @@ -33,7 +33,7 @@ function runTests(inputRoot: string, outputRoot: string, filter: (name: string) try { const node = parseJavaScript(input, {path: name}); - actual = transpileJavaScript(node, {id: "0", resolveImport: mockResolveImport}); + actual = transpileJavaScript(node, {id: "0", path: name, resolveImport: mockResolveImport}); } catch (error) { if (!(error instanceof SyntaxError)) throw error; actual = `define({id: "0", body: () => { throw new SyntaxError(${JSON.stringify(error.message)}); }});\n`; @@ -75,7 +75,7 @@ describe("transpileJavaScript(input, options)", () => { runTests("test/input/imports", "test/output/imports", (name) => name.endsWith("-import.js")); it("trims leading and trailing newlines", async () => { const node = parseJavaScript("\ntest\n", {path: "index.js"}); - const body = transpileJavaScript(node, {id: "0"}); + const body = transpileJavaScript(node, {id: "0", path: "index.js"}); assert.strictEqual(body, 'define({id: "0", inputs: ["test","display"], body: async (test,display) => {\ndisplay(await(\ntest\n))\n}});\n'); // prettier-ignore }); }); diff --git a/test/output/build/archives.posix/tar.html b/test/output/build/archives.posix/tar.html index 58f7e6faf..e5d190658 100644 --- a/test/output/build/archives.posix/tar.html +++ b/test/output/build/archives.posix/tar.html @@ -15,53 +15,53 @@ import {define} from "./_observablehq/client.js"; import {registerFile} from "./_observablehq/stdlib.js"; -registerFile("./dynamic-tar-gz/does-not-exist.txt", {"name":"./dynamic-tar-gz/does-not-exist.txt","mimeType":"text/plain","path":"./dynamic-tar-gz/does-not-exist.txt","lastModified":/* ts */1706742000000}); +registerFile("./dynamic-tar-gz/does-not-exist.txt", {"name":"./dynamic-tar-gz/does-not-exist.txt","mimeType":"text/plain","path":"./dynamic-tar-gz/does-not-exist.txt"}); registerFile("./dynamic-tar-gz/file.txt", {"name":"./dynamic-tar-gz/file.txt","mimeType":"text/plain","path":"./_file/dynamic-tar-gz/file.c93138d8.txt","lastModified":/* ts */1706742000000}); -registerFile("./dynamic-tar/does-not-exist.txt", {"name":"./dynamic-tar/does-not-exist.txt","mimeType":"text/plain","path":"./dynamic-tar/does-not-exist.txt","lastModified":/* ts */1706742000000}); +registerFile("./dynamic-tar/does-not-exist.txt", {"name":"./dynamic-tar/does-not-exist.txt","mimeType":"text/plain","path":"./dynamic-tar/does-not-exist.txt"}); registerFile("./dynamic-tar/file.txt", {"name":"./dynamic-tar/file.txt","mimeType":"text/plain","path":"./_file/dynamic-tar/file.c93138d8.txt","lastModified":/* ts */1706742000000}); -registerFile("./static-tar/does-not-exist.txt", {"name":"./static-tar/does-not-exist.txt","mimeType":"text/plain","path":"./static-tar/does-not-exist.txt","lastModified":/* ts */1706742000000}); +registerFile("./static-tar/does-not-exist.txt", {"name":"./static-tar/does-not-exist.txt","mimeType":"text/plain","path":"./static-tar/does-not-exist.txt"}); registerFile("./static-tar/file.txt", {"name":"./static-tar/file.txt","mimeType":"text/plain","path":"./_file/static-tar/file.c93138d8.txt","lastModified":/* ts */1706742000000}); registerFile("./static-tgz/file.txt", {"name":"./static-tgz/file.txt","mimeType":"text/plain","path":"./_file/static-tgz/file.c93138d8.txt","lastModified":/* ts */1706742000000}); define({id: "d5134368", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("static-tar/file.txt").text() +await FileAttachment("./static-tar/file.txt").text() )) }}); define({id: "a0c06958", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("static-tgz/file.txt").text() +await FileAttachment("./static-tgz/file.txt").text() )) }}); define({id: "d84cd7fb", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("static-tar/does-not-exist.txt").text() +await FileAttachment("./static-tar/does-not-exist.txt").text() )) }}); define({id: "86bd51aa", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic-tar/file.txt").text() +await FileAttachment("./dynamic-tar/file.txt").text() )) }}); define({id: "95938c22", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic-tar/does-not-exist.txt").text() +await FileAttachment("./dynamic-tar/does-not-exist.txt").text() )) }}); define({id: "7e5740fd", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic-tar-gz/file.txt").text() +await FileAttachment("./dynamic-tar-gz/file.txt").text() )) }}); define({id: "d0a58efd", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic-tar-gz/does-not-exist.txt").text() +await FileAttachment("./dynamic-tar-gz/does-not-exist.txt").text() )) }}); diff --git a/test/output/build/archives.posix/zip.html b/test/output/build/archives.posix/zip.html index 8ce30be01..66244a098 100644 --- a/test/output/build/archives.posix/zip.html +++ b/test/output/build/archives.posix/zip.html @@ -16,31 +16,31 @@ import {registerFile} from "./_observablehq/stdlib.js"; registerFile("./dynamic/file.txt", {"name":"./dynamic/file.txt","mimeType":"text/plain","path":"./_file/dynamic/file.c93138d8.txt","lastModified":/* ts */1706742000000}); -registerFile("./dynamic/not-found.txt", {"name":"./dynamic/not-found.txt","mimeType":"text/plain","path":"./dynamic/not-found.txt","lastModified":/* ts */1706742000000}); +registerFile("./dynamic/not-found.txt", {"name":"./dynamic/not-found.txt","mimeType":"text/plain","path":"./dynamic/not-found.txt"}); registerFile("./static/file.txt", {"name":"./static/file.txt","mimeType":"text/plain","path":"./_file/static/file.d9014c46.txt","lastModified":/* ts */1706742000000}); -registerFile("./static/not-found.txt", {"name":"./static/not-found.txt","mimeType":"text/plain","path":"./static/not-found.txt","lastModified":/* ts */1706742000000}); +registerFile("./static/not-found.txt", {"name":"./static/not-found.txt","mimeType":"text/plain","path":"./static/not-found.txt"}); define({id: "d3b9d0ee", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("static/file.txt").text() +await FileAttachment("./static/file.txt").text() )) }}); define({id: "bab54217", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("static/not-found.txt").text() +await FileAttachment("./static/not-found.txt").text() )) }}); define({id: "11eec300", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic/file.txt").text() +await FileAttachment("./dynamic/file.txt").text() )) }}); define({id: "ee2310f3", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -await FileAttachment("dynamic/not-found.txt").text() +await FileAttachment("./dynamic/not-found.txt").text() )) }}); diff --git a/test/output/build/files/files.html b/test/output/build/files/files.html index 2ef6532da..ef7f62456 100644 --- a/test/output/build/files/files.html +++ b/test/output/build/files/files.html @@ -22,25 +22,25 @@ define({id: "10037545", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("file-top.csv") +FileAttachment("./file-top.csv") )) }}); define({id: "453a8147", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("subsection/file-sub.csv") +FileAttachment("./subsection/file-sub.csv") )) }}); define({id: "444c421e", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("observable logo.png") +FileAttachment("./observable logo.png") )) }}); define({id: "cee3ab67", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("unknown-mime-extension.really") +FileAttachment("./unknown-mime-extension.really") )) }}); diff --git a/test/output/build/files/subsection/subfiles.html b/test/output/build/files/subsection/subfiles.html index 8170662ae..7f22815bf 100644 --- a/test/output/build/files/subsection/subfiles.html +++ b/test/output/build/files/subsection/subfiles.html @@ -26,7 +26,7 @@ define({id: "834ecf9f", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("file-sub.csv") +FileAttachment("./file-sub.csv") )) }}); diff --git a/test/output/build/imports/foo/foo.html b/test/output/build/imports/foo/foo.html index a6ff8013d..e179bb0fe 100644 --- a/test/output/build/imports/foo/foo.html +++ b/test/output/build/imports/foo/foo.html @@ -28,7 +28,7 @@ display(bar); display(top); -FileAttachment("/top.js"); +FileAttachment("../top.js"); return {d3,bar,top}; }}); diff --git a/test/output/build/missing-file/index.html b/test/output/build/missing-file/index.html index 0fd5e8250..c8bb854f8 100644 --- a/test/output/build/missing-file/index.html +++ b/test/output/build/missing-file/index.html @@ -19,7 +19,7 @@ define({id: "5760fd93", inputs: ["FileAttachment","display"], body: async (FileAttachment,display) => { display(await( -FileAttachment("does-not-exist.txt") +FileAttachment("./does-not-exist.txt") )) }}); diff --git a/test/output/build/multi/index.html b/test/output/build/multi/index.html index 64b13c4e1..814ca5f0f 100644 --- a/test/output/build/multi/index.html +++ b/test/output/build/multi/index.html @@ -20,7 +20,7 @@ registerFile("./file2.csv", {"name":"./file2.csv","mimeType":"text/csv","path":"./_file/file2.c70f7d51.csv","lastModified":/* ts */1706742000000}); define({id: "1bcb5df5", inputs: ["FileAttachment"], outputs: ["f1"], body: (FileAttachment) => { -const f1 = FileAttachment("file1.csv").csv(); +const f1 = FileAttachment("./file1.csv").csv(); return {f1}; }}); diff --git a/test/output/build/simple/simple.html b/test/output/build/simple/simple.html index 520cc47e3..b370e429a 100644 --- a/test/output/build/simple/simple.html +++ b/test/output/build/simple/simple.html @@ -18,7 +18,7 @@ registerFile("./data.txt", {"name":"./data.txt","mimeType":"text/plain","path":"./_file/data.9d63c3b5.txt","lastModified":/* ts */1706742000000}); define({id: "115586ff", inputs: ["FileAttachment"], outputs: ["result"], body: (FileAttachment) => { -let result = FileAttachment("data.txt").text(); +let result = FileAttachment("./data.txt").text(); return {result}; }}); diff --git a/test/output/template-file-attachment.js b/test/output/template-file-attachment.js index 9691d09ca..d8d57634d 100644 --- a/test/output/template-file-attachment.js +++ b/test/output/template-file-attachment.js @@ -1,3 +1,3 @@ define({id: "0", inputs: ["FileAttachment"], body: (FileAttachment) => { -FileAttachment(`test.js`); +FileAttachment("./test.js"); }});