Skip to content

Commit 3ef99bc

Browse files
authored
fix transitive static imports (#980)
1 parent 97aa3ce commit 3ef99bc

File tree

8 files changed

+120
-40
lines changed

8 files changed

+120
-40
lines changed

src/build.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ export async function build(
7272
const options = {path, ...config};
7373
effects.output.write(`${faint("parse")} ${sourcePath} `);
7474
const start = performance.now();
75-
const page = await parseMarkdown(sourcePath, options);
75+
const source = await readFile(sourcePath, "utf8");
76+
const page = parseMarkdown(source, options);
7677
if (page?.data?.draft) {
7778
effects.logger.log(faint("(skipped)"));
7879
continue;

src/config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {existsSync} from "node:fs";
2+
import {readFile} from "node:fs/promises";
23
import op from "node:path";
34
import {basename, dirname, join} from "node:path/posix";
45
import {cwd} from "node:process";
@@ -82,7 +83,8 @@ async function readPages(root: string): Promise<Page[]> {
8283
const pages: Page[] = [];
8384
for await (const file of visitMarkdownFiles(root)) {
8485
if (file === "index.md" || file === "404.md") continue;
85-
const parsed = await parseMarkdown(join(root, file), {root, path: file});
86+
const source = await readFile(join(root, file), "utf8");
87+
const parsed = parseMarkdown(source, {root, path: file});
8688
if (parsed?.data?.draft) continue;
8789
const name = basename(file, ".md");
8890
const page = {path: join("/", dirname(file), name), name: parsed.title ?? "Untitled"};

src/markdown.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
/* eslint-disable import/no-named-as-default-member */
22
import {createHash} from "node:crypto";
3-
import {readFile} from "node:fs/promises";
43
import matter from "gray-matter";
54
import he from "he";
65
import MarkdownIt from "markdown-it";
@@ -276,12 +275,11 @@ export interface ParseOptions {
276275
style?: Config["style"];
277276
}
278277

279-
export async function parseMarkdown(
280-
sourcePath: string,
278+
export function parseMarkdown(
279+
input: string,
281280
{root, path, markdownIt = (md) => md, style: configStyle}: ParseOptions
282-
): Promise<MarkdownPage> {
283-
const source = await readFile(sourcePath, "utf-8");
284-
const parts = matter(source, {});
281+
): MarkdownPage {
282+
const parts = matter(input, {});
285283
const md = markdownIt(MarkdownIt({html: true}));
286284
md.use(MarkdownItAnchor, {permalink: MarkdownItAnchor.permalink.headerLink({class: "observablehq-header-anchor"})});
287285
md.inline.ruler.push("placeholder", transformPlaceholderInline);

src/preview.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ export class PreviewServer {
200200
// Anything else should 404; static files should be matched above.
201201
try {
202202
const options = {path: pathname, ...config, preview: true};
203-
const parse = await parseMarkdown(path + ".md", options);
203+
const source = await readFile(path + ".md", "utf8");
204+
const parse = parseMarkdown(source, options);
204205
const html = await renderPage(parse, options);
205206
end(req, res, html, "text/html");
206207
} catch (error) {
@@ -218,7 +219,8 @@ export class PreviewServer {
218219
if (req.method === "GET" && res.statusCode === 404) {
219220
try {
220221
const options = {path: "/404", ...config, preview: true};
221-
const parse = await parseMarkdown(join(root, "404.md"), options);
222+
const source = await readFile(join(root, "404.md"), "utf8");
223+
const parse = parseMarkdown(source, options);
222224
const html = await renderPage(parse, options);
223225
end(req, res, html, "text/html");
224226
return;
@@ -312,7 +314,8 @@ function handleWatch(socket: WebSocket, req: IncomingMessage, config: Config) {
312314
break;
313315
}
314316
case "change": {
315-
const page = await parseMarkdown(join(root, path), {path, ...config});
317+
const source = await readFile(join(root, path), "utf8");
318+
const page = parseMarkdown(source, {path, ...config});
316319
// delay to avoid a possibly-empty file
317320
if (!force && page.html === "") {
318321
if (!emptyTimeout) {
@@ -361,7 +364,8 @@ function handleWatch(socket: WebSocket, req: IncomingMessage, config: Config) {
361364
if (!(path = normalize(path)).startsWith("/")) throw new Error("Invalid path: " + initialPath);
362365
if (path.endsWith("/")) path += "index";
363366
path += ".md";
364-
const page = await parseMarkdown(join(root, path), {path, ...config});
367+
const source = await readFile(join(root, path), "utf8");
368+
const page = parseMarkdown(source, {path, ...config});
365369
const resolvers = await getResolvers(page, {root, path});
366370
if (resolvers.hash !== initialHash) return void send({type: "reload"});
367371
hash = resolvers.hash;

src/resolvers.ts

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -102,32 +102,27 @@ export async function getResolvers(page: MarkdownPage, {root, path}: {root: stri
102102
for (const i of localImports) hash.update(getModuleHash(root, resolvePath(path, i)));
103103
if (page.style && isPathImport(page.style)) hash.update(getFileHash(root, resolvePath(path, page.style)));
104104

105-
// Collect transitively-attached files and imports.
105+
// Collect transitively-attached files and local imports.
106106
for (const i of localImports) {
107107
const p = resolvePath(path, i);
108108
const info = getModuleInfo(root, p);
109109
if (!info) continue;
110-
for (const f of info.files) {
111-
files.add(relativePath(path, resolvePath(p, f)));
112-
}
113-
for (const m of info.fileMethods) {
114-
fileMethods.add(m);
115-
}
116-
for (const o of info.localStaticImports) {
117-
const r = relativePath(path, resolvePath(p, o));
118-
localImports.add(r);
119-
staticImports.add(r);
120-
}
121-
for (const o of info.localDynamicImports) {
122-
localImports.add(relativePath(path, resolvePath(p, o)));
123-
}
124-
for (const o of info.globalStaticImports) {
125-
staticImports.add(o);
126-
globalImports.add(o);
127-
}
128-
for (const o of info.globalDynamicImports) {
129-
globalImports.add(o);
130-
}
110+
for (const f of info.files) files.add(relativePath(path, resolvePath(p, f)));
111+
for (const m of info.fileMethods) fileMethods.add(m);
112+
for (const o of info.localStaticImports) localImports.add(relativePath(path, resolvePath(p, o)));
113+
for (const o of info.localDynamicImports) localImports.add(relativePath(path, resolvePath(p, o)));
114+
for (const o of info.globalStaticImports) globalImports.add(o);
115+
for (const o of info.globalDynamicImports) globalImports.add(o);
116+
}
117+
118+
// Collect static imports from transitive local imports.
119+
for (const i of staticImports) {
120+
if (!localImports.has(i)) continue;
121+
const p = resolvePath(path, i);
122+
const info = getModuleInfo(root, p);
123+
if (!info) continue;
124+
for (const o of info.localStaticImports) staticImports.add(relativePath(path, resolvePath(p, o)));
125+
for (const o of info.globalStaticImports) staticImports.add(o);
131126
}
132127

133128
// Add implicit imports for files. These are technically implemented as
@@ -219,8 +214,6 @@ export async function getResolvers(page: MarkdownPage, {root, path}: {root: stri
219214
? relativePath(path, `/_observablehq/${specifier.slice("observablehq:".length)}${extname(specifier) ? "" : ".js"}`) // prettier-ignore
220215
: resolutions.has(specifier)
221216
? relativePath(path, resolutions.get(specifier)!)
222-
: specifier.startsWith("npm:")
223-
? `https://cdn.jsdelivr.net/npm/${specifier.slice("npm:".length)}`
224217
: specifier;
225218
}
226219

@@ -235,8 +228,6 @@ export async function getResolvers(page: MarkdownPage, {root, path}: {root: stri
235228
? relativePath(path, `/_observablehq/${specifier.slice("observablehq:".length)}`)
236229
: resolutions.has(specifier)
237230
? relativePath(path, resolutions.get(specifier)!)
238-
: specifier.startsWith("npm:")
239-
? `https://cdn.jsdelivr.net/npm/${specifier.slice("npm:".length)}`
240231
: specifier;
241232
}
242233

src/search.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {readFile} from "node:fs/promises";
12
import {basename, join} from "node:path/posix";
23
import he from "he";
34
import MiniSearch from "minisearch";
@@ -41,7 +42,8 @@ export async function searchIndex(config: Config, effects = defaultEffects): Pro
4142
const index = new MiniSearch(indexOptions);
4243
for await (const file of visitMarkdownFiles(root)) {
4344
const path = join(root, file);
44-
const {html, title, data} = await parseMarkdown(path, {root, path: "/" + file.slice(0, -3)});
45+
const source = await readFile(path, "utf8");
46+
const {html, title, data} = parseMarkdown(source, {root, path: "/" + file.slice(0, -3)});
4547

4648
// Skip pages that opt-out of indexing, and skip unlisted pages unless
4749
// opted-in. We only log the first case.

test/markdown-test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ describe("parseMarkdown(input)", () => {
1919
const outname = only || skip ? name.slice(5) : name;
2020

2121
(only ? it.only : skip ? it.skip : it)(`test/input/${name}`, async () => {
22-
const snapshot = await parseMarkdown(path, {root: "test/input", path: name});
22+
const source = await readFile(path, "utf8");
23+
const snapshot = parseMarkdown(source, {root: "test/input", path: name});
2324
let allequal = true;
2425
for (const ext of ["html", "json"]) {
2526
const actual = ext === "json" ? jsonMeta(snapshot) : snapshot[ext];

test/resolvers-test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import assert from "node:assert";
2+
import {parseMarkdown} from "../src/markdown.js";
3+
import {getResolvers} from "../src/resolvers.js";
4+
5+
describe("getResolvers(page, {root, path})", () => {
6+
const builtins = ["npm:@observablehq/runtime", "npm:@observablehq/stdlib", "observablehq:client"];
7+
it("resolves directly-attached files", async () => {
8+
const options = {root: "test/input", path: "attached.md"};
9+
const page = parseMarkdown("${FileAttachment('foo.csv')}", options);
10+
const resolvers = await getResolvers(page, options);
11+
assert.deepStrictEqual(resolvers.files, new Set(["./foo.csv"]));
12+
});
13+
it("ignores files that are outside of the source root", async () => {
14+
const options = {root: "test/input", path: "attached.md"};
15+
const page = parseMarkdown("${FileAttachment('../foo.csv')}", options);
16+
const resolvers = await getResolvers(page, options);
17+
assert.deepStrictEqual(resolvers.files, new Set([]));
18+
});
19+
it("detects file methods", async () => {
20+
const options = {root: "test/input", path: "attached.md"};
21+
const page = parseMarkdown("${FileAttachment('foo.csv').csv}", options);
22+
const resolvers = await getResolvers(page, options);
23+
assert.deepStrictEqual(resolvers.staticImports, new Set(["npm:d3-dsv", ...builtins]));
24+
});
25+
it("detects local static imports", async () => {
26+
const options = {root: "test/input/imports", path: "attached.md"};
27+
const page = parseMarkdown("```js\nimport './bar.js';\n```", options);
28+
const resolvers = await getResolvers(page, options);
29+
assert.deepStrictEqual(resolvers.staticImports, new Set(["./bar.js", ...builtins]));
30+
assert.deepStrictEqual(resolvers.localImports, new Set(["./bar.js"]));
31+
});
32+
it("detects local transitive static imports", async () => {
33+
const options = {root: "test/input/imports", path: "attached.md"};
34+
const page = parseMarkdown("```js\nimport './other/foo.js';\n```", options);
35+
const resolvers = await getResolvers(page, options);
36+
assert.deepStrictEqual(resolvers.staticImports, new Set(["./other/foo.js", "./bar.js", ...builtins]));
37+
assert.deepStrictEqual(resolvers.localImports, new Set(["./other/foo.js", "./bar.js"]));
38+
});
39+
it("detects local transitive static imports (2)", async () => {
40+
const options = {root: "test/input/imports", path: "attached.md"};
41+
const page = parseMarkdown("```js\nimport './transitive-static-import.js';\n```", options);
42+
const resolvers = await getResolvers(page, options);
43+
assert.deepStrictEqual(resolvers.staticImports, new Set(["./transitive-static-import.js", "./other/foo.js", "./bar.js", ...builtins])); // prettier-ignore
44+
assert.deepStrictEqual(resolvers.localImports, new Set(["./transitive-static-import.js", "./other/foo.js", "./bar.js"])); // prettier-ignore
45+
});
46+
it("detects local transitive dynamic imports", async () => {
47+
const options = {root: "test/input/imports", path: "attached.md"};
48+
const page = parseMarkdown("```js\nimport './dynamic-import.js';\n```", options);
49+
const resolvers = await getResolvers(page, options);
50+
assert.deepStrictEqual(resolvers.staticImports, new Set(["./dynamic-import.js", ...builtins]));
51+
assert.deepStrictEqual(resolvers.localImports, new Set(["./dynamic-import.js", "./bar.js"]));
52+
});
53+
it("detects local transitive dynamic imports (2)", async () => {
54+
const options = {root: "test/input/imports", path: "attached.md"};
55+
const page = parseMarkdown("```js\nimport('./dynamic-import.js');\n```", options);
56+
const resolvers = await getResolvers(page, options);
57+
assert.deepStrictEqual(resolvers.staticImports, new Set(builtins));
58+
assert.deepStrictEqual(resolvers.localImports, new Set(["./dynamic-import.js", "./bar.js"]));
59+
});
60+
it("detects local transitive dynamic imports (3)", async () => {
61+
const options = {root: "test/input/imports", path: "attached.md"};
62+
const page = parseMarkdown("```js\nimport('./transitive-dynamic-import.js');\n```", options);
63+
const resolvers = await getResolvers(page, options);
64+
assert.deepStrictEqual(resolvers.staticImports, new Set(builtins));
65+
assert.deepStrictEqual(resolvers.localImports, new Set(["./transitive-dynamic-import.js", "./other/foo.js", "./bar.js"])); // prettier-ignore
66+
});
67+
it("detects local transitive dynamic imports (4)", async () => {
68+
const options = {root: "test/input/imports", path: "attached.md"};
69+
const page = parseMarkdown("```js\nimport('./transitive-static-import.js');\n```", options);
70+
const resolvers = await getResolvers(page, options);
71+
assert.deepStrictEqual(resolvers.staticImports, new Set(builtins));
72+
assert.deepStrictEqual(resolvers.localImports, new Set(["./transitive-static-import.js", "./other/foo.js", "./bar.js"])); // prettier-ignore
73+
});
74+
it("detects local dynamic imports", async () => {
75+
const options = {root: "test/input", path: "attached.md"};
76+
const page = parseMarkdown("${import('./foo.js')}", options);
77+
const resolvers = await getResolvers(page, options);
78+
assert.deepStrictEqual(resolvers.staticImports, new Set(builtins));
79+
assert.deepStrictEqual(resolvers.localImports, new Set(["./foo.js"]));
80+
});
81+
});

0 commit comments

Comments
 (0)