Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix file path resolution #120

Merged
merged 21 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix file path resolution
  • Loading branch information
mbostock committed Nov 8, 2023
commit 931d740cd8665febae80257826fdb8fa2f885715
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
},
"dependencies": {
"@observablehq/runtime": "^5.9.4",
"acorn": "^8.10.0",
"acorn-walk": "^8.2.0",
"acorn": "^8.11.2",
"acorn-walk": "^8.3.0",
"fast-array-diff": "^1.1.0",
"fast-deep-equal": "^3.1.3",
"gray-matter": "^4.0.3",
Expand Down
2 changes: 1 addition & 1 deletion public/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export function define(cell) {
v.define(outputs.length ? `cell ${id}` : null, inputs, body);
variables.push(v);
for (const o of outputs) variables.push(main.define(o, [`cell ${id}`], (exports) => exports[o]));
for (const f of files) attachedFiles.set(f.name, {url: String(new URL(`/_file/${f.name}`, location)), mimeType: f.mimeType}); // prettier-ignore
for (const f of files) attachedFiles.set(f.name, {url: `/_file${(new URL(f.name, location)).pathname}`, mimeType: f.mimeType}); // prettier-ignore
for (const d of databases) databaseTokens.set(d.name, d);
}

Expand Down
4 changes: 2 additions & 2 deletions src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ async function build(context: CommandContext) {
const sourcePath = join(sourceRoot, sourceFile);
const outputPath = join(outputRoot, join(dirname(sourceFile), basename(sourceFile, ".md") + ".html"));
console.log("render", sourcePath, "→", outputPath);
const path = `/${join(dirname(sourceFile), basename(sourceFile, ".md"))}`;
const path = join(dirname(sourceFile), basename(sourceFile, ".md"));
const render = renderServerless(await readFile(sourcePath, "utf-8"), {
root: sourceRoot,
path,
pages,
resolver
});
files.push(...render.files.map((f) => f.name));
files.push(...render.files.map((f) => join(dirname(sourceFile), f.name)));
await prepareOutput(outputPath);
await writeFile(outputPath, render.html);
}
Expand Down
27 changes: 19 additions & 8 deletions src/javascript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ export interface ImportReference {
type: "global" | "local";
}

export interface Feature {
type: "FileAttachment" | "DatabaseClient" | "Secret";
name: string;
}

export interface Identifier {
name: string;
}

export interface Transpile {
id: string;
inputs?: string[];
Expand All @@ -47,10 +56,12 @@ export function transpileJavaScript(input: string, options: ParseOptions): Trans
const {id, root, sourcePath} = options;
try {
const node = parseJavaScript(input, options);
const databases = node.features.filter((f) => f.type === "DatabaseClient").map((f) => ({name: f.name}));
const databases = node.features
.filter((f) => f.type === "DatabaseClient")
.map((f): DatabaseReference => ({name: f.name}));
const files = node.features
.filter((f) => f.type === "FileAttachment")
.map((f) => ({name: f.name, mimeType: mime.getType(f.name)}));
.map((f): FileReference => ({name: f.name, mimeType: mime.getType(f.name)}));
const inputs = Array.from(new Set<string>(node.references.map((r) => r.name)));
const output = new Sourcemap(input);
trim(output, input);
Expand Down Expand Up @@ -103,12 +114,12 @@ export const parseOptions: Options = {ecmaVersion: 13, sourceType: "module"};

export interface JavaScriptNode {
body: Node;
declarations: {name: string}[] | null;
references: {name: string}[];
features: {type: unknown; name: string}[];
imports: {type: "global" | "local"; name: string}[];
expression: boolean;
async: boolean;
declarations: Identifier[] | null; // null for expressions that can’t declare top-level variables, a.k.a outputs
references: Identifier[]; // the unbound references, a.k.a. inputs
features: Feature[];
imports: ImportReference[];
expression: boolean; // is this an expression or a program cell?
async: boolean; // does this use top-level await?
}

function parseJavaScript(input: string, options: ParseOptions): JavaScriptNode {
Expand Down
10 changes: 6 additions & 4 deletions src/javascript/features.js → src/javascript/features.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {simple} from "acorn-walk";
import {type Feature} from "../javascript.js";
import {isLocalImport} from "./imports.js";
import {syntaxError} from "./syntaxError.js";
import {isLocalImport} from "./imports.ts";
import {dirname, join} from "node:path";

export function findFeatures(node, root, sourcePath, references, input) {
const features = [];
const features: Feature[] = [];

simple(node, {
CallExpression(node) {
Expand All @@ -13,9 +13,10 @@ export function findFeatures(node, root, sourcePath, references, input) {
arguments: args,
arguments: [arg]
} = node;

// Promote fetches with static literals to file attachment references.
if (isLocalFetch(node, references, root, sourcePath)) {
features.push({type: "FileAttachment", name: join(dirname(sourcePath), getStringLiteralValue(arg))});
features.push({type: "FileAttachment", name: getStringLiteralValue(arg)});
return;
}

Expand All @@ -34,6 +35,7 @@ export function findFeatures(node, root, sourcePath, references, input) {
if (args.length !== 1 || !isStringLiteral(arg)) {
throw syntaxError(`${callee.name} requires a single literal string argument`, node, input);
}

features.push({type: callee.name, name: getStringLiteralValue(arg)});
}
});
Expand Down
File renamed without changes.
File renamed without changes.
9 changes: 4 additions & 5 deletions src/javascript/imports.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import {Parser} from "acorn";
import type {Node} from "acorn";
import {Parser, type Node} from "acorn";
import {simple} from "acorn-walk";
import {readFileSync} from "node:fs";
import {dirname, join} from "node:path";
import {type JavaScriptNode, parseOptions} from "../javascript.js";
import {parseOptions, type Feature, type ImportReference, type JavaScriptNode} from "../javascript.js";
import {getStringLiteralValue, isStringLiteral} from "./features.js";

export function findImports(body: Node, root: string, sourcePath: string) {
const imports: {name: string; type: "global" | "local"}[] = [];
const features: {name: string; type: string}[] = [];
const imports: ImportReference[] = [];
const features: Feature[] = [];
const paths = new Set<string>();

simple(body, {
Expand Down
20 changes: 11 additions & 9 deletions src/markdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import {type RuleInline} from "markdown-it/lib/parser_inline.js";
import {type default as Renderer, type RenderRule} from "markdown-it/lib/renderer.js";
import mime from "mime";
import {readFile} from "node:fs/promises";
import {pathFromRoot} from "./files.js";
import {isLocalFile, pathFromRoot} from "./files.js";
import {computeHash} from "./hash.js";
import {transpileJavaScript, type FileReference, type ImportReference, type Transpile} from "./javascript.js";
import {transpileTag} from "./tag.js";
import {dirname, join} from "node:path";

export interface ReadMarkdownResult {
contents: string;
Expand Down Expand Up @@ -51,7 +52,7 @@ interface RenderPiece {

interface ParseContext {
pieces: RenderPiece[];
files: {name: string; mimeType: string | null}[];
files: FileReference[];
imports: ImportReference[];
startLine: number;
currentLine: number;
Expand Down Expand Up @@ -286,7 +287,7 @@ function extendPiece(context: ParseContext, extend: Partial<RenderPiece>) {
};
}

function renderIntoPieces(renderer: Renderer, root: string): Renderer["render"] {
function renderIntoPieces(renderer: Renderer, root: string, sourcePath: string): Renderer["render"] {
return (tokens, options, context: ParseContext) => {
const rules = renderer.rules;
for (let i = 0, len = tokens.length; i < len; i++) {
Expand All @@ -306,7 +307,7 @@ function renderIntoPieces(renderer: Renderer, root: string): Renderer["render"]
}
let result = "";
for (const piece of context.pieces) {
result += piece.html = normalizePieceHtml(piece.html, root, context);
result += piece.html = normalizePieceHtml(piece.html, root, sourcePath, context);
}
return result;
};
Expand All @@ -316,13 +317,14 @@ function renderIntoPieces(renderer: Renderer, root: string): Renderer["render"]
// stylesheets), this ensures that the HTML for each piece generates exactly one
// top-level element. This is necessary for incremental update, and ensures that
// our parsing of the Markdown is consistent with the resulting HTML structure.
function normalizePieceHtml(html: string, root: string, context: ParseContext): string {
function normalizePieceHtml(html: string, root: string, sourcePath: string, context: ParseContext): string {
const {document} = parseHTML(html);
for (const element of document.querySelectorAll("link[href]") as any as Iterable<Element>) {
const href = pathFromRoot(element.getAttribute("href"), root);
if (href) {
const href = element.getAttribute("href")!;
const path = join(dirname(sourcePath), href);
if (isLocalFile(path, root)) {
context.files.push({name: href, mimeType: mime.getType(href)});
element.setAttribute("href", `/_file/${href}`);
element.setAttribute("href", `/_file/${path}`);
}
}
return isSingleElement(document) ? String(document) : `<span>${document}</span>`;
Expand Down Expand Up @@ -373,7 +375,7 @@ export function parseMarkdown(source: string, root: string, sourcePath: string):
md.renderer.rules.placeholder = makePlaceholderRenderer(root, sourcePath);
md.renderer.rules.fence = makeFenceRenderer(root, md.renderer.rules.fence!, sourcePath);
md.renderer.rules.softbreak = makeSoftbreakRenderer(md.renderer.rules.softbreak!);
md.renderer.render = renderIntoPieces(md.renderer, root);
md.renderer.render = renderIntoPieces(md.renderer, root, sourcePath);
const context: ParseContext = {files: [], imports: [], pieces: [], startLine: 0, currentLine: 0};
const tokens = md.parse(parts.content, context);
const html = md.renderer.render(tokens, md.options, context); // Note: mutates context.pieces, context.files!
Expand Down
13 changes: 7 additions & 6 deletions src/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {WebSocketServer, type WebSocket} from "ws";
import {findLoader, runLoader} from "./dataloader.js";
import {HttpError, isHttpError, isNodeError} from "./error.js";
import {maybeStat} from "./files.js";
import {type FileReference} from "./javascript.js";
import {diffMarkdown, readMarkdown, type ParseResult} from "./markdown.js";
import {readPages} from "./navigation.js";
import {renderPreview} from "./render.js";
Expand Down Expand Up @@ -81,7 +82,7 @@ class Server {
}
throw new HttpError("Not found", 404);
} else {
if (normalize(pathname).startsWith("..")) throw new Error("Invalid path: " + pathname);
if ((pathname = normalize(pathname)).startsWith("..")) throw new Error("Invalid path: " + pathname);
mbostock marked this conversation as resolved.
Show resolved Hide resolved
let path = join(this.root, pathname);

// If this path is for /index, redirect to the parent directory for a
Expand Down Expand Up @@ -120,7 +121,7 @@ class Server {
(
await renderPreview(await readFile(path + ".md", "utf-8"), {
root: this.root,
path: pathname,
path: pathname.slice("/".length),
pages,
resolver: this._resolver!
})
Expand Down Expand Up @@ -151,9 +152,9 @@ class Server {
class FileWatchers {
#watchers: FSWatcher[] = [];

static async watchAll(root: string, files: {name: string}[], cb: (name: string) => void) {
static async watchAll(root: string, files: FileReference[], cb: (name: string) => void) {
const watchers = new FileWatchers();
const fileset = [...new Set(files.map(({name}) => name))];
const fileset = [...new Set(files.map(({name}) => name))]; // TODO need resolved files here, relative to dirname(sourcePath)
for (const name of fileset) {
const watchPath = await FileWatchers.getWatchPath(root, name);
let prevState = await maybeStat(watchPath);
Expand Down Expand Up @@ -250,9 +251,9 @@ function handleWatch(socket: WebSocket, options: {root: string; resolver: CellRe
case "hello": {
if (markdownWatcher || attachmentWatcher) throw new Error("already watching");
let {path} = message;
if (normalize(path).startsWith("..")) throw new Error("Invalid path: " + path);
if (!(path = normalize(path)).startsWith("/")) throw new Error("Invalid path: " + path);
if (path.endsWith("/")) path += "index";
path = normalize(path) + ".md";
path = path.slice("/".length) + ".md";
markdownWatcher = watch(join(root, path), await refreshMarkdown(path));
break;
}
Expand Down
2 changes: 1 addition & 1 deletion test/javascript-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function runTests({
await transpileJavaScript(await readFile(path, "utf8"), {
id: "0",
root: inputRoot,
sourcePath: "/"
sourcePath: name
})
);
let expected;
Expand Down
2 changes: 1 addition & 1 deletion test/markdown-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe("parseMarkdown(input)", () => {
const outname = only || skip ? name.slice(5) : name;

(only ? it.only : skip ? it.skip : it)(`test/input/${name}`, async () => {
const snapshot = parseMarkdown(await readFile(path, "utf8"), "test/input", "/");
const snapshot = parseMarkdown(await readFile(path, "utf8"), "test/input", name);
let allequal = true;
for (const ext of ["html", "json"]) {
const actual = ext === "json" ? jsonMeta(snapshot) : snapshot[ext];
Expand Down
2 changes: 1 addition & 1 deletion test/output/imports/dynamic-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define({id: "0", outputs: ["foo"], files: [{"name":"/bar.js","mimeType":"application/javascript"}], body: async () => {
define({id: "0", outputs: ["foo"], files: [{"name":"bar.js","mimeType":"application/javascript"}], body: async () => {
const foo = await import("/_file/bar.js");
return {foo};
}});
2 changes: 1 addition & 1 deletion test/output/imports/static-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define({id: "0", inputs: ["display"], outputs: ["foo"], files: [{"name":"/bar.js","mimeType":"application/javascript"}], body: async (display) => {
define({id: "0", inputs: ["display"], outputs: ["foo"], files: [{"name":"bar.js","mimeType":"application/javascript"}], body: async (display) => {
const {foo} = await import("/_file/bar.js");

display(foo);
Expand Down
2 changes: 1 addition & 1 deletion test/output/imports/transitive-dynamic-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define({id: "0", outputs: ["foo"], files: [{"name":"/other/foo.js","mimeType":"application/javascript"},{"name":"/bar.js","mimeType":"application/javascript"}], body: async () => {
define({id: "0", outputs: ["foo"], files: [{"name":"other/foo.js","mimeType":"application/javascript"},{"name":"bar.js","mimeType":"application/javascript"}], body: async () => {
const foo = await import("/_file/other/foo.js");
return {foo};
}});
2 changes: 1 addition & 1 deletion test/output/imports/transitive-static-import.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define({id: "0", outputs: ["foo"], files: [{"name":"/other/foo.js","mimeType":"application/javascript"},{"name":"/bar.js","mimeType":"application/javascript"}], body: async () => {
define({id: "0", outputs: ["foo"], files: [{"name":"other/foo.js","mimeType":"application/javascript"},{"name":"bar.js","mimeType":"application/javascript"}], body: async () => {
const {foo} = await import("/_file/other/foo.js");
return {foo};
}});
4 changes: 2 additions & 2 deletions test/output/local-fetch.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"title": "Local fetch",
"files": [
{
"name": "/local-fetch.md",
"name": "./local-fetch.md",
"mimeType": "text/markdown"
}
],
Expand Down Expand Up @@ -33,7 +33,7 @@
],
"files": [
{
"name": "/local-fetch.md",
"name": "./local-fetch.md",
"mimeType": "text/markdown"
}
],
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -385,15 +385,15 @@ acorn-jsx@^5.3.2:
resolved "https://registry.yarnpkg.com/acorn-jsx/-/acorn-jsx-5.3.2.tgz#7ed5bb55908b3b2f1bc55c6af1653bada7f07937"
integrity sha512-rq9s+JNhf0IChjtDXxllJ7g41oZk5SlXtp0LHwyA5cejwn7vKmKp4pPri6YEePv2PU65sAsegbXtIinmDFDXgQ==

acorn-walk@^8.2.0:
version "8.2.0"
resolved "https://registry.yarnpkg.com/acorn-walk/-/acorn-walk-8.2.0.tgz#741210f2e2426454508853a2f44d0ab83b7f69c1"
integrity sha512-k+iyHEuPgSw6SbuDpGQM+06HQUa04DZ3o+F6CSzXMvvI5KMvnaEqXe+YVe555R9nn6GPt404fos4wcgpw12SDA==

acorn@^8.10.0, acorn@^8.9.0:
version "8.10.0"
resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.10.0.tgz#8be5b3907a67221a81ab23c7889c4c5526b62ec5"
integrity sha512-F0SAmZ8iUtS//m8DmCTA0jlh6TDKkHQyK6xc6V4KDTyZKA9dnvX9/3sRTVQrWm79glUAZbnmmNcdYwUIHWVybw==
acorn-walk@^8.3.0:
version "8.3.0"
resolved "https://registry.yarnpkg.com/acorn-walk/-/acorn-walk-8.3.0.tgz#2097665af50fd0cf7a2dfccd2b9368964e66540f"
integrity sha512-FS7hV565M5l1R08MXqo8odwMTB02C2UqzB17RVgu9EyuYFBqJZ3/ZY97sQD5FewVu1UyDFc1yztUDrAwT0EypA==

acorn@^8.11.2, acorn@^8.9.0:
version "8.11.2"
resolved "https://registry.yarnpkg.com/acorn/-/acorn-8.11.2.tgz#ca0d78b51895be5390a5903c5b3bdcdaf78ae40b"
integrity sha512-nc0Axzp/0FILLEVsm4fNwLCwMttvhEI263QtVPQcbpfZZ3ts0hLsZGOpE6czNlid7CJ9MlyH8reXkpsf3YUY4w==

ajv@^6.12.4:
version "6.12.6"
Expand Down