From 00298f5fc5ba5609db89cc2a7cb5212cf0c82abd Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 3 Nov 2023 17:38:06 -0700 Subject: [PATCH 1/4] code style --- src/build.ts | 13 ++++---- src/dataloader.ts | 80 ++++++++++++++++++++++------------------------- src/files.ts | 8 ++--- src/preview.ts | 50 +++++++++++++---------------- src/render.ts | 8 +++-- src/resolver.ts | 13 ++++---- 6 files changed, 81 insertions(+), 91 deletions(-) diff --git a/src/build.ts b/src/build.ts index ba2dcf095..756fcd13a 100644 --- a/src/build.ts +++ b/src/build.ts @@ -3,11 +3,11 @@ import {basename, dirname, join, normalize, relative} from "node:path"; import {cwd} from "node:process"; import {fileURLToPath} from "node:url"; import {parseArgs} from "node:util"; -import {getStats, prepareOutput, visitFiles, visitMarkdownFiles} from "./files.js"; +import {maybeLoader, runLoader} from "./dataloader.js"; +import {maybeStat, prepareOutput, visitFiles, visitMarkdownFiles} from "./files.js"; import {readPages} from "./navigation.js"; import {renderServerless} from "./render.js"; import {makeCLIResolver} from "./resolver.js"; -import {findLoader, runCommand} from "./dataloader.js"; const EXTRA_FILES = new Map([["node_modules/@observablehq/runtime/dist/runtime.js", "_observablehq/runtime.js"]]); @@ -53,15 +53,16 @@ async function build(context: CommandContext) { for (const file of files) { const sourcePath = join(sourceRoot, file); const outputPath = join(outputRoot, "_file", file); - const stats = await getStats(sourcePath); + const stats = await maybeStat(sourcePath); if (!stats) { - const {path} = await findLoader("", sourcePath); - if (!path) { + const loader = await maybeLoader(sourcePath); + if (!loader) { console.error("missing referenced file", sourcePath); continue; } + const {path} = loader; console.log("generate", path, "→", outputPath); - await runCommand(path, outputPath); + await runLoader(path, outputPath); continue; } console.log("copy", sourcePath, "→", outputPath); diff --git a/src/dataloader.ts b/src/dataloader.ts index 756be8a09..d93e81352 100644 --- a/src/dataloader.ts +++ b/src/dataloader.ts @@ -1,57 +1,53 @@ -import {open} from "node:fs/promises"; import {spawn} from "node:child_process"; -import {join} from "node:path"; -import {getStats, prepareOutput} from "./files.js"; -import {renameSync, unlinkSync} from "node:fs"; +import {type Stats} from "node:fs"; +import {open, rename, unlink} from "node:fs/promises"; +import {maybeStat, prepareOutput} from "./files.js"; const runningCommands = new Map>(); -export async function runCommand(commandPath: string, outputPath: string) { +export interface Loader { + path: string; + stats: Stats; +} + +export async function runLoader(commandPath: string, outputPath: string) { if (runningCommands.has(commandPath)) return runningCommands.get(commandPath); - const command = new Promise((resolve, reject) => { + const command = (async () => { const outputTempPath = outputPath + ".tmp"; - prepareOutput(outputTempPath).then(() => - open(outputTempPath, "w").then((cacheFd) => { - const cacheFileStream = cacheFd.createWriteStream({highWaterMark: 1024 * 1024}); - try { - const subprocess = spawn(commandPath, [], { - argv0: commandPath, - //cwd: dirname(commandPath), // TODO: Need to change commandPath to be relative this? - windowsHide: true, - stdio: ["ignore", "pipe", "inherit"] - // timeout: // time in ms - // signal: // abort signal - }); - subprocess.stdout.on("data", (data) => cacheFileStream.write(data)); - subprocess.on("error", (error) => console.error(`${commandPath}: ${error.message}`)); - subprocess.on("close", (code) => { - cacheFd.close().then(() => { - if (code === 0) { - renameSync(outputTempPath, outputPath); - } else { - unlinkSync(outputTempPath); - } - resolve(); - }, reject); - }); - } catch (error) { - reject(error); - } finally { - runningCommands.delete(commandPath); - } - }) - ); - }); + await prepareOutput(outputTempPath); + const cacheFd = await open(outputTempPath, "w"); + const cacheFileStream = cacheFd.createWriteStream({highWaterMark: 1024 * 1024}); + const subprocess = spawn(commandPath, [], { + argv0: commandPath, + //cwd: dirname(commandPath), // TODO: Need to change commandPath to be relative this? + windowsHide: true, + stdio: ["ignore", "pipe", "inherit"] + // timeout: // time in ms + // signal: // abort signal + }); + subprocess.stdout.pipe(cacheFileStream); + const code = await new Promise((resolve, reject) => { + subprocess.on("error", reject); // (error) => console.error(`${commandPath}: ${error.message}`)); + subprocess.on("close", resolve); + }); + await cacheFd.close(); + if (code === 0) { + await rename(outputTempPath, outputPath); + } else { + await unlink(outputTempPath); + } + })(); + command.finally(() => runningCommands.delete(commandPath)); runningCommands.set(commandPath, command); return command; } -export async function findLoader(root: string, name: string) { +export async function maybeLoader(name: string): Promise { // TODO: It may be more efficient use fs.readdir for (const ext of [".js", ".ts", ".sh"]) { - const path = join(root, name) + ext; - const stats = await getStats(path); + const path = name + ext; + const stats = await maybeStat(path); if (stats) return {path, stats}; } - return {}; + return null; } diff --git a/src/files.ts b/src/files.ts index 4ba8074f5..acea096df 100644 --- a/src/files.ts +++ b/src/files.ts @@ -1,5 +1,4 @@ -import type {Stats} from "node:fs"; -import {accessSync, constants, statSync} from "node:fs"; +import {accessSync, constants, statSync, type Stats} from "node:fs"; import {mkdir, readdir, stat} from "node:fs/promises"; import {dirname, extname, join, normalize, relative} from "node:path"; import {isNodeError} from "./error.js"; @@ -52,13 +51,14 @@ export async function* visitFiles(root: string): AsyncGenerator { } } -export async function getStats(path: string): Promise { +// Like fs.stat, but returns null instead of throwing ENOENT if not found. +export async function maybeStat(path: string): Promise { try { return await stat(path); } catch (error) { if (!isNodeError(error) || error.code !== "ENOENT") throw error; } - return; + return null; } export async function prepareOutput(outputPath: string): Promise { diff --git a/src/preview.ts b/src/preview.ts index 5dd07c08b..42a23d334 100644 --- a/src/preview.ts +++ b/src/preview.ts @@ -1,5 +1,4 @@ -import type {WatchListener} from "node:fs"; -import {watch, type FSWatcher} from "node:fs"; +import {watch, type FSWatcher, type WatchListener} from "node:fs"; import {access, constants, readFile, stat} from "node:fs/promises"; import {createServer, type IncomingMessage, type RequestListener} from "node:http"; import {basename, dirname, extname, join, normalize} from "node:path"; @@ -7,15 +6,13 @@ import {fileURLToPath} from "node:url"; import {parseArgs} from "node:util"; import send from "send"; import {WebSocketServer, type WebSocket} from "ws"; +import {maybeLoader, runLoader} from "./dataloader.js"; import {HttpError, isHttpError, isNodeError} from "./error.js"; -import type {ParseResult} from "./markdown.js"; -import {diffMarkdown, readMarkdown} from "./markdown.js"; +import {maybeStat} from "./files.js"; +import {diffMarkdown, readMarkdown, type ParseResult} from "./markdown.js"; import {readPages} from "./navigation.js"; import {renderPreview} from "./render.js"; -import type {CellResolver} from "./resolver.js"; -import {makeCLIResolver} from "./resolver.js"; -import {findLoader, runCommand} from "./dataloader.js"; -import {getStats} from "./files.js"; +import {makeCLIResolver, type CellResolver} from "./resolver.js"; const publicRoot = join(dirname(fileURLToPath(import.meta.url)), "..", "public"); const cacheRoot = join(dirname(fileURLToPath(import.meta.url)), "..", ".observablehq", "cache"); @@ -70,18 +67,18 @@ class Server { } // Look for a data loader for this file. - const {path: loaderPath, stats: loaderStat} = await findLoader(this.root, path); - if (loaderStat) { + const loader = await maybeLoader(filepath); + if (loader) { const cachePath = join(this.cacheRoot, filepath); - const cacheStat = await getStats(cachePath); - if (cacheStat && cacheStat.mtimeMs > loaderStat.mtimeMs) { + const cacheStat = await maybeStat(cachePath); + if (cacheStat && cacheStat.mtimeMs > loader.stats.mtimeMs) { send(req, filepath, {root: this.cacheRoot}).pipe(res); return; } - if (!(loaderStat.mode & constants.S_IXUSR)) { + if (!(loader.stats.mode & constants.S_IXUSR)) { throw new HttpError("Data loader is not executable", 404); } - await runCommand(loaderPath, cachePath); + await runLoader(loader.path, cachePath); send(req, filepath, {root: this.cacheRoot}).pipe(res); return; } @@ -167,10 +164,10 @@ class FileWatchers { const fileset = [...new Set(this.files.map(({name}) => name))]; for (const name of fileset) { const watchPath = await FileWatchers.getWatchPath(this.root, name); - let prevState = await getStats(watchPath); + let prevState = await maybeStat(watchPath); this.watchers.push( watch(watchPath, async () => { - const newState = await getStats(watchPath); + const newState = await maybeStat(watchPath); // Ignore if the file was truncated or not modified. if (prevState?.mtimeMs === newState?.mtimeMs || newState?.size === 0) return; prevState = newState; @@ -182,10 +179,10 @@ class FileWatchers { static async getWatchPath(root: string, name: string) { const path = join(root, name); - const stats = await getStats(path); + const stats = await maybeStat(path); if (stats?.isFile()) return path; - const {path: loaderPath, stats: loaderStat} = await findLoader(root, name); - return loaderStat?.isFile() ? loaderPath : path; + const loader = await maybeLoader(path); + return loader?.stats.isFile() ? loader.path : path; } close() { @@ -195,16 +192,11 @@ class FileWatchers { } function resolveDiffs(diff: ReturnType, resolver: CellResolver): ReturnType { - for (const item of diff) { - if (item.type === "add") { - for (const addItem of item.items) { - if (addItem.type === "cell" && "databases" in addItem) { - Object.assign(addItem, resolver(addItem)); - } - } - } - } - return diff; + return diff.map((item) => + item.type === "add" + ? {...item, items: item.items.map((addItem) => (addItem.type === "cell" ? resolver(addItem) : addItem))} + : item + ); } function handleWatch(socket: WebSocket, options: {root: string; resolver: CellResolver}) { diff --git a/src/render.ts b/src/render.ts index 49f297323..7611ffbd9 100644 --- a/src/render.ts +++ b/src/render.ts @@ -1,8 +1,7 @@ import {computeHash} from "./hash.js"; import {type FileReference, type ImportReference} from "./javascript.js"; import {resolveImport} from "./javascript/imports.js"; -import type {CellPiece} from "./markdown.js"; -import {parseMarkdown, type ParseResult} from "./markdown.js"; +import {parseMarkdown, type CellPiece, type ParseResult} from "./markdown.js"; export interface Render { html: string; @@ -60,7 +59,10 @@ ${ } ${Array.from(getImportPreloads(parseResult)) - .concat(parseResult.imports.filter(({name}) => name.startsWith("./")).map(({name}) => `/_file/${name.slice(2)}`)) + .concat( + parseResult.imports.filter(({name}) => name.startsWith("./")).map(({name}) => `/_file/${name.slice(2)}`), + parseResult.cells.some((cell) => cell.databases?.length) ? "/_observablehq/database.js" : [] + ) .map((href) => ``) .join("\n")}