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

annotate local paths #1731

Merged
merged 18 commits into from
Oct 28, 2024
29 changes: 17 additions & 12 deletions src/javascript/transpile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export async function transpileModule(

async function rewriteImportSource(source: StringLiteral) {
const specifier = getStringLiteralValue(source);
output.replaceLeft(source.start, source.end, JSON.stringify(await resolveImport(specifier)));
output.replaceLeft(source.start, source.end, annotatePath(await resolveImport(specifier)));
}

for (const {name, node} of findFiles(body, path, input)) {
Expand All @@ -111,17 +111,15 @@ export async function transpileModule(
output.replaceLeft(
source.start,
source.end,
`${JSON.stringify(
`${
info
? {
name: p,
mimeType: mime.getType(name) ?? undefined,
path: relativePath(servePath, resolveFile(name)),
lastModified: info.mtimeMs,
size: info.size
}
: p
)}, import.meta.url`
? `{"name":${JSON.stringify(p)},"mimeType":${JSON.stringify(
mime.getType(name) ?? undefined
)},"path":${annotatePath(relativePath(servePath, resolveFile(name)))},"lastModified":${JSON.stringify(
info.mtimeMs
)},"size":${JSON.stringify(info.size)}`
Fil marked this conversation as resolved.
Show resolved Hide resolved
: JSON.stringify(p)
}, import.meta.url`
);
}

Expand All @@ -137,7 +135,7 @@ export async function transpileModule(
if (isImportMetaResolve(node) && isStringLiteral(source)) {
const value = getStringLiteralValue(source);
const resolution = isPathImport(value) && !isJavaScript(value) ? resolveFile(value) : await resolveImport(value);
output.replaceLeft(source.start, source.end, JSON.stringify(resolution));
output.replaceLeft(source.start, source.end, annotatePath(resolution));
}
}

Expand Down Expand Up @@ -259,3 +257,10 @@ export function rewriteParams(output: Sourcemap, body: Node, params: Params, inp
output.replaceLeft(node.start, node.end, JSON.stringify(params[name]));
}
}

/**
* Annotate a path to a local import or file so it can be reworked server-side.
*/
function annotatePath(uri: string): string {
return `${JSON.stringify(uri)}${isPathImport(uri) ? "/* observablehq-file */" : ""}`;
}
4 changes: 2 additions & 2 deletions test/javascript/transpile-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ describe("transpileModule(input, root, path)", () => {
it("rewrites npm imports", async () => {
const input = 'import "npm:d3-array";';
const output = (await transpileModule(input, options)).split("\n").pop()!;
assert.strictEqual(output, 'import "../_npm/[email protected]/_esm.js";');
assert.strictEqual(output, 'import "../_npm/[email protected]/_esm.js"/* observablehq-file */;');
Copy link
Member

Choose a reason for hiding this comment

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

If this link is tagged as observable-file, then we should also rewrite it to tag Observable files in it. If we don't rewrite it, I don't think we should tag it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was initially doing that (see line 307 in 6cfb74d#diff-a479e6903ca67c840829983998e33849155911842cf08fa118b42b2d88fe3c9b), then I think we decided to tag everything and let the server decide what it should do with it.

Copy link
Member

Choose a reason for hiding this comment

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

My concern is that this is a reference to an _npm file that is tagged, but references to _npm files from other _npm files are not tagged. As a concrete example, in the docs build there is this chain:

  1. dist/chart.js tags its import to the _import file:
  2. docs/.observablehq/dist/_import/chart.95e875cf.js tags its import of an _npm file:
  3. docs/.observablehq/dist/_npm/@observablehq/[email protected]/e828d8c8.js does not tag its import of the _npm file:
  4. ../../[email protected]/c68fbd73.js

It would be helpful if the tagging was done based on the taget of the URL, not the place the URL occurs. As it stands, the server can't really make the decision fully, because links to _npm files aren't consistent tagged/not tagged, so it has to special case them as not needing extra authentication.

});
it("rewrites node imports", async () => {
const input = 'import "d3-array";';
const output = (await transpileModule(input, options)).split("\n").pop()!;
assert.strictEqual(output, 'import "../_node/[email protected]/index.js";');
assert.strictEqual(output, 'import "../_node/[email protected]/index.js"/* observablehq-file */;');
});
});
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export {test} from "./test.86a60bc6.js";
export {test} from "./test.86a60bc6.js"/* observablehq-file */;
4 changes: 2 additions & 2 deletions test/output/build/data-loaders/_import/test.86a60bc6.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import {FileAttachment} from "../_observablehq/stdlib.00000003.js";
import {FileAttachment} from "../_observablehq/stdlib.00000003.js"/* observablehq-file */;

export const test = FileAttachment({"name":"../test.txt","mimeType":"text/plain","path":"../_file/test.f2ca1bb6.txt","lastModified":/* ts */1706742000000,"size":5}, import.meta.url).text();
export const test = FileAttachment({"name":"../test.txt","mimeType":"text/plain","path":"../_file/test.f2ca1bb6.txt"/* observablehq-file */,"lastModified":/* ts */1706742000000,"size":5, import.meta.url).text();
Fil marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 3 additions & 3 deletions test/output/build/embed/_import/chart.2ce91e05.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {FileAttachment} from "../_observablehq/stdlib.00000003.js";
import * as Plot from "../_npm/@observablehq/[email protected]/cd372fb8.js";
import {FileAttachment} from "../_observablehq/stdlib.00000003.js"/* observablehq-file */;
import * as Plot from "../_npm/@observablehq/[email protected]/cd372fb8.js"/* observablehq-file */;

export async function Chart() {
const gistemp = await FileAttachment({"name":"../lib/gistemp.csv","mimeType":"text/csv","path":"../_file/lib/gistemp.1cf298b1.csv","lastModified":/* ts */1706742000000,"size":97}, import.meta.url).csv({typed: true});
const gistemp = await FileAttachment({"name":"../lib/gistemp.csv","mimeType":"text/csv","path":"../_file/lib/gistemp.1cf298b1.csv"/* observablehq-file */,"lastModified":/* ts */1706742000000,"size":97, import.meta.url).csv({typed: true});
return Plot.plot({
y: {grid: true},
color: {scheme: "burd"},
Expand Down
8 changes: 4 additions & 4 deletions test/output/build/embed/chart.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import "./_observablehq/stdlib.00000003.js";
import "./_npm/@observablehq/[email protected]/cd372fb8.js";
import "./_npm/[email protected]/cd372fb8.js";
export * from "./_import/chart.2ce91e05.js";
import "./_observablehq/stdlib.00000003.js"/* observablehq-file */;
import "./_npm/@observablehq/[email protected]/cd372fb8.js"/* observablehq-file */;
import "./_npm/[email protected]/cd372fb8.js"/* observablehq-file */;
export * from "./_import/chart.2ce91e05.js"/* observablehq-file */;
6 changes: 3 additions & 3 deletions test/output/build/fetches/_import/foo/foo.666599bc.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions test/output/build/fetches/_import/top.c85e149a.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/output/build/imports/_import/bar/bar.13bb8056.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export {bar} from "./baz.cdbfb28b.js";
export {bar} from "./baz.cdbfb28b.js"/* observablehq-file */;
2 changes: 1 addition & 1 deletion test/output/build/imports/_import/bar/baz.cdbfb28b.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {foo} from "../foo/foo.5963bf78.js";
import {foo} from "../foo/foo.5963bf78.js"/* observablehq-file */;

export const bar = "bar";
export const foobar = foo + "bar";
6 changes: 3 additions & 3 deletions test/output/build/imports/_import/foo/foo.5963bf78.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import "../../_npm/[email protected]/cd372fb8.js";
import {bar} from "../bar/bar.13bb8056.js";
export {top} from "../top.160847a6.js";
import "../../_npm/[email protected]/cd372fb8.js"/* observablehq-file */;
import {bar} from "../bar/bar.13bb8056.js"/* observablehq-file */;
export {top} from "../top.160847a6.js"/* observablehq-file */;

export const foo = "foo";
export const foobar = "foo" + bar;
2 changes: 1 addition & 1 deletion test/output/build/npm/_import/index.4bdc071f.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
import "../_observablehq/stdlib/inputs.00000005.js";
import "../_observablehq/stdlib/inputs.00000005.js"/* observablehq-file */;