Skip to content

build with relative paths #173

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

Merged
merged 10 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion public/client.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Inspector, Library, Runtime} from "/_observablehq/runtime.js";
import {Inspector, Library, Runtime} from "./runtime.js";

const library = Object.assign(new Library(), {width, Mutable, ...recommendedLibraries()});
const runtime = new Runtime(library);
Expand Down
2 changes: 1 addition & 1 deletion src/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {isNodeError} from "./error.js";

// A path is local if it doesn’t go outside the the root.
export function getLocalPath(sourcePath: string, name: string): string | null {
if (/^(\w+:)\/\//.test(name)) return null; // URL
if (/^\w+:/.test(name)) return null; // URL
const path = join(dirname(sourcePath.startsWith("/") ? sourcePath.slice("/".length) : sourcePath), name);
if (path.startsWith("../")) return null; // goes above root
return path;
Expand Down
11 changes: 7 additions & 4 deletions src/javascript/imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {dirname, join, normalize} from "node:path";
import {type ExportAllDeclaration, type ExportNamedDeclaration, type Node, Parser} from "acorn";
import {simple} from "acorn-walk";
import {type ImportReference, type JavaScriptNode, parseOptions} from "../javascript.js";
import {relativeUrl} from "../url.js";
import {getStringLiteralValue, isStringLiteral} from "./features.js";

export function findExports(body: Node) {
Expand Down Expand Up @@ -86,7 +87,7 @@ export function rewriteImports(output: any, rootNode: JavaScriptNode, sourcePath
node.source.end,
JSON.stringify(
isLocalImport(value, sourcePath)
? join("/_import/", join(dirname(sourcePath), value))
? relativeUrl(sourcePath, join("/_import/", dirname(sourcePath), value))
: resolveImport(value)
)
);
Expand All @@ -104,16 +105,18 @@ export function rewriteImports(output: any, rootNode: JavaScriptNode, sourcePath
? `{${node.specifiers.filter(isNotNamespaceSpecifier).map(rewriteImportSpecifier).join(", ")}}`
: node.specifiers.find(isNamespaceSpecifier)?.local.name ?? "{}"
} = await import(${JSON.stringify(
isLocalImport(value, sourcePath)
? join("/_import/", join(dirname(sourcePath), value))
: resolveImport(value)
isLocalImport(value, sourcePath) ? relativeImport(sourcePath, value) : resolveImport(value)
)});`
);
}
}
});
}

function relativeImport(sourcePath, value) {
return relativeUrl(sourcePath, join("/_import/", dirname(sourcePath), value));
}

function rewriteImportSpecifier(node) {
return node.type === "ImportDefaultSpecifier"
? `default: ${node.local.name}`
Expand Down
13 changes: 8 additions & 5 deletions src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {computeHash} from "./hash.js";
import {resolveImport} from "./javascript/imports.js";
import {type FileReference, type ImportReference} from "./javascript.js";
import {type CellPiece, type ParseResult, parseMarkdown} from "./markdown.js";
import {relativeUrl} from "./url.js";

export interface Render {
html: string;
Expand Down Expand Up @@ -64,13 +65,13 @@ ${
: ""
}<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="stylesheet" type="text/css" href="${relativeUrl(path, "/_observablehq/style.css")}">
${Array.from(getImportPreloads(parseResult, path))
.map((href) => `<link rel="modulepreload" href="${href}">`)
.map((href) => `<link rel="modulepreload" href="${relativeUrl(path, href)}">`)
.join("\n")}
<script type="module">

import {${preview ? "open, " : ""}define} from "/_observablehq/client.js";
import {${preview ? "open, " : ""}define} from "${relativeUrl(path, "/_observablehq/client.js")}";

${preview ? `open({hash: ${JSON.stringify(hash)}});\n` : ""}${parseResult.cells
.map(resolver)
Expand All @@ -92,7 +93,7 @@ ${
? `
<ol>
<li class="observablehq-link">
<a href="/">${escapeData(title)}</a>
<a href="${relativeUrl(path, "/")}">${escapeData(title)}</a>
</li>
</ol>`
: ""
Expand Down Expand Up @@ -148,7 +149,9 @@ ${parseResult.html}</main>
function renderListItem(p: Page, path: string): string {
return `<li class="observablehq-link${
p.path === path ? " observablehq-link-active" : ""
}"><a href="${escapeDoubleQuoted(p.path.replace(/\/index$/, "") || "/")}">${escapeData(p.name)}</a></li>`;
}"><a href="${escapeDoubleQuoted(relativeUrl(path, p.path.replace(/\/index$/, "") || "/"))}">${escapeData(
p.name
)}</a></li>`;
}

function getImportPreloads(parseResult: ParseResult, path: string): Iterable<string> {
Expand Down
15 changes: 15 additions & 0 deletions src/url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Returns the relative path from "/file/path/to/a" to "/file/path/of/b". To
// make relative imports work, paths to the same directory are prefixed with
// "./", and paths that start without a slash are considered from the root.
export function relativeUrl(source, target) {
if (/^\w+:/.test(target)) return target;
const from = `/${source}`.split(/[/]+/g).slice(0, -1);
const to = `/${target}`.split(/[/]+/g);
Comment on lines +6 to +7
Copy link
Member

@mbostock mbostock Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s worth mentioning that relativeUrl can be called with two different types of paths with different characteristics: serving paths and source paths.

Serving paths, such as during render, start with a slash and don’t end with an extension. For example /javascript/displays.

Source paths, such as during rewriteImports, don’t start with a slash and do end with an extension. For example, javascript/displays.md. Source paths are typically relative to the source root, or to a subdirectory in source root (such as when computing transitive imports).

It looks like the code already handles both of these correctly, but I think it’s worth calling it out in the tests, and having explicit tests for each type of path. I think we also want more tests for the source path case.

const f = to.pop()!;
const m = from.length;
const n = Math.min(m, to.length);
let i = 0;
while (i < n && from[i] === to[i]) ++i;
const k = m - i;
return (k ? "../".repeat(k) : "./") + to.slice(i).concat(f).join("/");
}
12 changes: 6 additions & 6 deletions test/output/build/config/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
<title>Index</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="./_observablehq/style.css">
<link rel="modulepreload" href="./_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "./_observablehq/client.js";


</script>
<input id="observablehq-sidebar-toggle" type="checkbox">
<nav id="observablehq-sidebar">
<ol>
<li class="observablehq-link observablehq-link-active"><a href="/">Index</a></li>
<li class="observablehq-link"><a href="/one">One</a></li>
<li class="observablehq-link"><a href="/sub/two">Two</a></li>
<li class="observablehq-link observablehq-link-active"><a href="./">Index</a></li>
<li class="observablehq-link"><a href="./one">One</a></li>
<li class="observablehq-link"><a href="./sub/two">Two</a></li>
</ol>
</nav>
<script>{
Expand Down
12 changes: 6 additions & 6 deletions test/output/build/config/one.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
<title>One</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="./_observablehq/style.css">
<link rel="modulepreload" href="./_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "./_observablehq/client.js";


</script>
<input id="observablehq-sidebar-toggle" type="checkbox">
<nav id="observablehq-sidebar">
<ol>
<li class="observablehq-link"><a href="/">Index</a></li>
<li class="observablehq-link observablehq-link-active"><a href="/one">One</a></li>
<li class="observablehq-link"><a href="/sub/two">Two</a></li>
<li class="observablehq-link"><a href="./">Index</a></li>
<li class="observablehq-link observablehq-link-active"><a href="./one">One</a></li>
<li class="observablehq-link"><a href="./sub/two">Two</a></li>
</ol>
</nav>
<script>{
Expand Down
12 changes: 6 additions & 6 deletions test/output/build/config/sub/two.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
<title>Two</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="../_observablehq/style.css">
<link rel="modulepreload" href="../_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "../_observablehq/client.js";


</script>
<input id="observablehq-sidebar-toggle" type="checkbox">
<nav id="observablehq-sidebar">
<ol>
<li class="observablehq-link"><a href="/">Index</a></li>
<li class="observablehq-link"><a href="/one">One</a></li>
<li class="observablehq-link observablehq-link-active"><a href="/sub/two">Two</a></li>
<li class="observablehq-link"><a href="../">Index</a></li>
<li class="observablehq-link"><a href="../one">One</a></li>
<li class="observablehq-link observablehq-link-active"><a href="./two">Two</a></li>
</ol>
</nav>
<script>{
Expand Down
14 changes: 7 additions & 7 deletions test/output/build/imports/foo/foo.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
<title>Foo</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="../_observablehq/style.css">
<link rel="modulepreload" href="../_observablehq/runtime.js">
<link rel="modulepreload" href="https://cdn.jsdelivr.net/npm/d3/+esm">
<link rel="modulepreload" href="/_import/bar/bar.js">
<link rel="modulepreload" href="/_import/bar/baz.js">
<link rel="modulepreload" href="/_import/foo/foo.js">
<link rel="modulepreload" href="../_import/bar/bar.js">
<link rel="modulepreload" href="../_import/bar/baz.js">
<link rel="modulepreload" href="../_import/foo/foo.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "../_observablehq/client.js";

define({id: "a9220fae", inputs: ["display"], outputs: ["d3","bar"], body: async (display) => {
const d3 = await import("https://cdn.jsdelivr.net/npm/d3/+esm");
const {bar} = await import("/_import/bar/bar.js");
const {bar} = await import("../_import/bar/bar.js");

display(bar);
return {d3,bar};
Expand Down
10 changes: 5 additions & 5 deletions test/output/build/multi/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
<title>Multi test</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="./_observablehq/style.css">
<link rel="modulepreload" href="./_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "./_observablehq/client.js";

define({id: "1bcb5df5", inputs: ["FileAttachment"], outputs: ["f1"], files: [{"name":"file1.csv","mimeType":"text/csv"}], body: (FileAttachment) => {
const f1 = FileAttachment("file1.csv").csv();
Expand All @@ -28,8 +28,8 @@
<input id="observablehq-sidebar-toggle" type="checkbox">
<nav id="observablehq-sidebar">
<ol>
<li class="observablehq-link"><a href="/subsection">Sub-Section</a></li>
<li class="observablehq-link observablehq-link-active"><a href="/">Multi test</a></li>
<li class="observablehq-link"><a href="./subsection">Sub-Section</a></li>
<li class="observablehq-link observablehq-link-active"><a href="./">Multi test</a></li>
</ol>
</nav>
<script>{
Expand Down
10 changes: 5 additions & 5 deletions test/output/build/multi/subsection/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
<title>Sub-Section</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="../_observablehq/style.css">
<link rel="modulepreload" href="../_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "../_observablehq/client.js";


</script>
<input id="observablehq-sidebar-toggle" type="checkbox">
<nav id="observablehq-sidebar">
<ol>
<li class="observablehq-link observablehq-link-active"><a href="/subsection">Sub-Section</a></li>
<li class="observablehq-link"><a href="/">Multi test</a></li>
<li class="observablehq-link observablehq-link-active"><a href="../subsection">Sub-Section</a></li>
<li class="observablehq-link"><a href="../">Multi test</a></li>
</ol>
</nav>
<script>{
Expand Down
6 changes: 3 additions & 3 deletions test/output/build/simple/simple.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
<title>Build test case</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
<link rel="stylesheet" type="text/css" href="/_observablehq/style.css">
<link rel="modulepreload" href="/_observablehq/runtime.js">
<link rel="stylesheet" type="text/css" href="./_observablehq/style.css">
<link rel="modulepreload" href="./_observablehq/runtime.js">
<script type="module">

import {define} from "/_observablehq/client.js";
import {define} from "./_observablehq/client.js";

define({id: "115586ff", inputs: ["FileAttachment"], outputs: ["result"], files: [{"name":"data.txt","mimeType":"text/plain"}], body: (FileAttachment) => {
let result = FileAttachment("data.txt").text();
Expand Down
2 changes: 1 addition & 1 deletion test/output/dynamic-import-noent.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
define({id: "0", outputs: ["foo"], body: async () => {
const foo = await import("/_import/noent.js");
const foo = await import("./_import/noent.js");
return {foo};
}});
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"], body: async () => {
const foo = await import("/_import/bar.js");
const foo = await import("./_import/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,5 +1,5 @@
define({id: "0", inputs: ["display"], outputs: ["foo"], body: async (display) => {
const {foo} = await import("/_import/bar.js");
const {foo} = await import("./_import/bar.js");

display(foo);
return {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"], body: async () => {
const foo = await import("/_import/other/foo.js");
const foo = await import("./_import/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"], body: async () => {
const {foo} = await import("/_import/other/foo.js");
const {foo} = await import("./_import/other/foo.js");
return {foo};
}});
2 changes: 1 addition & 1 deletion test/output/static-import-noent.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
define({id: "0", inputs: ["display"], outputs: ["foo"], body: async (display) => {
const {foo} = await import("/_import/noent.js");
const {foo} = await import("./_import/noent.js");

display(foo);
return {foo};
Expand Down
40 changes: 40 additions & 0 deletions test/url-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import assert from "node:assert";
import {relativeUrl} from "../src/url.js";

describe("relativeUrls", () => {
it("respects absolute links", () => {
assert.strictEqual(relativeUrl("/", "https://whatever"), "https://whatever");
assert.strictEqual(relativeUrl("/", "http://example.org"), "http://example.org");
assert.strictEqual(relativeUrl("/", "https://example.org/"), "https://example.org/");
assert.strictEqual(relativeUrl("/", "mailto:hello@example.org"), "mailto:hello@example.org");
});
it("return the expected result", () => {
assert.strictEqual(relativeUrl("/", "/"), "./");
assert.strictEqual(relativeUrl("/foo", "/"), "./");
assert.strictEqual(relativeUrl("/foo.html", "/"), "./");
assert.strictEqual(relativeUrl("/", "/foo"), "./foo");
assert.strictEqual(relativeUrl("/", "/foo.html"), "./foo.html");
assert.strictEqual(relativeUrl("/", "/foo/bar/baz"), "./foo/bar/baz");
assert.strictEqual(relativeUrl("/foo", "/foo"), "./foo");
assert.strictEqual(relativeUrl("/foo/", "/foo/"), "./");
assert.strictEqual(relativeUrl("/foo", "/foo/"), "./foo/");
Comment on lines +19 to +20
Copy link
Contributor

@cinxmo cinxmo Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain the difference between source being /foo/ vs /foo?
/foo/ means you're in a directory under /foo vs in /foo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we should make the differentiation between "/foo" vs "/foo/". What is the use case for /foo/ without the nested directory (like /foo/bar). Would the target ever have a trailing slash?

Copy link
Contributor Author

@Fil Fil Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's when you have docs/features/index.md, vs docs/features.md; see #171 #172 and #175 for discussion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get /foo/ if you have a foo/index.md. You get /foo if you have a foo.md.

assert.strictEqual(relativeUrl("/foo/", "/foo"), "../foo");
assert.strictEqual(relativeUrl("/foo/bar", "/foo/bar"), "./bar");
assert.strictEqual(relativeUrl("/foo/bar/", "/foo/bar/"), "./");
assert.strictEqual(relativeUrl("/foo/bar", "/foo/bar/"), "./bar/");
assert.strictEqual(relativeUrl("/foo/bar/", "/foo/bar"), "../bar");
assert.strictEqual(relativeUrl("/foo", "/bar"), "./bar");
assert.strictEqual(relativeUrl("/foo/bar", "/baz"), "../baz");
assert.strictEqual(relativeUrl("/foo/bar", "/foo"), "../foo");
assert.strictEqual(relativeUrl("/foo/bar", "/foo.csv"), "../foo.csv");
assert.strictEqual(relativeUrl("/foo/bar", "/foo/"), "./");
assert.strictEqual(relativeUrl("/foo/bar", "/baz/bar"), "../baz/bar");
assert.strictEqual(relativeUrl("foo", "bar"), "./bar");
assert.strictEqual(relativeUrl("foo/bar", "baz"), "../baz");
assert.strictEqual(relativeUrl("foo/bar", "foo"), "../foo");
assert.strictEqual(relativeUrl("foo/bar", "foo.csv"), "../foo.csv");
assert.strictEqual(relativeUrl("foo/bar", "foo/"), "./");
assert.strictEqual(relativeUrl("foo/bar", "baz/bar"), "../baz/bar");
assert.strictEqual(relativeUrl("foo////baz", "baz//bar"), "../baz/bar");
});
});