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
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@
"test": "concurrently npm:test:mocha npm:test:tsc npm:test:lint npm:test:prettier",
"test:coverage": "c8 --check-coverage --lines 80 --per-file yarn test:mocha",
"test:build": "rimraf test/build && cross-env npm_package_version=1.0.0-test node build.js --sourcemap --outdir=test/build \"{src,test}/**/*.{ts,js,css}\" --ignore \"test/input/**\" --ignore \"test/output/**\" --ignore \"test/preview/dashboard/**\" --ignore \"**/*.d.ts\" && cp -r templates test/build",
"test:mocha": "yarn test:build && rimraf --glob test/.observablehq/cache test/input/build/*/.observablehq/cache && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 TZ=America/Los_Angeles mocha --timeout 30000 -p \"test/build/test/**/*-test.js\"",
"test:mocha:serial": "yarn test:build && rimraf --glob test/.observablehq/cache test/input/build/*/.observablehq/cache && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/*-test.js\"",
"test:mocha": "yarn test:build && rimraf --glob test/.observablehq/cache test/input/build/*/.observablehq/cache && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 TZ=America/Los_Angeles mocha --timeout 30000 -p \"test/build/test/**/*-test.js\" && yarn test:annotate",
"test:mocha:serial": "yarn test:build && rimraf --glob test/.observablehq/cache test/input/build/*/.observablehq/cache && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/*-test.js\" && yarn test:annotate",
"test:annotate": "yarn test:build && cross-env OBSERVABLE_ANNOTATE_FILES=true TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/annotate.js\"",
Copy link
Member

Choose a reason for hiding this comment

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

This pattern breaks yarn test:mocha:serial with it.only or describe.only because it always runs the test:annotate in addition to your only’d tests.

"test:lint": "eslint src test --max-warnings=0",
"test:prettier": "prettier --check src test",
"test:tsc": "tsc --noEmit",
Expand Down
14 changes: 14 additions & 0 deletions src/javascript/annotate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {isPathImport} from "../path.js";

/**
* Annotate a path to a local import or file so it can be reworked server-side.
*/

const annotate = process.env["OBSERVABLE_ANNOTATE_FILES"];
if (typeof annotate === "string" && annotate !== "true")
throw new Error(`unsupported OBSERVABLE_ANNOTATE_FILES value: ${annotate}`);
export default annotate
Copy link
Member

Choose a reason for hiding this comment

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

We don’t use default exports (because there’s no real reason to, and it encourages more inconsistency in how people name an imported symbol). This should be called annotatePath or something.

? function (uri: string): string {
return `${JSON.stringify(uri)}${isPathImport(uri) ? "/* observablehq-file */" : ""}`;
}
: JSON.stringify;
25 changes: 12 additions & 13 deletions src/javascript/transpile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {isPathImport, relativePath, resolvePath, resolveRelativePath} from "../p
import {getModuleResolver} from "../resolvers.js";
import type {Params} from "../route.js";
import {Sourcemap} from "../sourcemap.js";
import annotate from "./annotate.js";
import type {FileExpression} from "./files.js";
import {findFiles} from "./files.js";
import type {ExportNode, ImportNode} from "./imports.js";
Expand Down Expand Up @@ -101,7 +102,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, annotate(await resolveImport(specifier)));
}

for (const {name, node} of findFiles(body, path, input)) {
Expand All @@ -111,17 +112,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":${annotate(relativePath(servePath, resolveFile(name)))},"lastModified":${JSON.stringify(
info.mtimeMs
)},"size":${JSON.stringify(info.size)}}`
: JSON.stringify(p)
}, import.meta.url`
);
}

Expand All @@ -137,7 +136,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, annotate(resolution));
}
}

Expand Down Expand Up @@ -205,7 +204,7 @@ function rewriteImportDeclarations(
for (const node of declarations) {
output.delete(node.start, node.end + +(output.input[node.end] === "\n"));
specifiers.push(rewriteImportSpecifiers(node));
imports.push(`import(${JSON.stringify(resolve(getStringLiteralValue(node.source as StringLiteral)))})`);
imports.push(`import(${annotate(resolve(getStringLiteralValue(node.source as StringLiteral)))})`);
}
if (declarations.length > 1) {
output.insertLeft(0, `const [${specifiers.join(", ")}] = await Promise.all([${imports.join(", ")}]);\n`);
Expand Down
3 changes: 2 additions & 1 deletion src/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {AstNode, OutputChunk, Plugin, ResolveIdResult} from "rollup";
import {rollup} from "rollup";
import esbuild from "rollup-plugin-esbuild";
import {prepareOutput, toOsPath} from "./files.js";
import annotate from "./javascript/annotate.js";
import type {ImportReference} from "./javascript/imports.js";
import {isJavaScript, parseImports} from "./javascript/imports.js";
import {parseNpmSpecifier, rewriteNpmImports} from "./npm.js";
Expand Down Expand Up @@ -86,7 +87,7 @@ function isBadCommonJs(specifier: string): boolean {
}

function shimCommonJs(specifier: string, require: NodeRequire): string {
return `export {${Object.keys(require(specifier))}} from ${JSON.stringify(specifier)};\n`;
return `export {${Object.keys(require(specifier))}} from ${annotate(specifier)};\n`;
}

async function bundle(
Expand Down
3 changes: 2 additions & 1 deletion src/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {CallExpression} from "acorn";
import {simple} from "acorn-walk";
import {maxSatisfying, rsort, satisfies, validRange} from "semver";
import {isEnoent} from "./error.js";
import annotate from "./javascript/annotate.js";
import type {ExportNode, ImportNode, ImportReference} from "./javascript/imports.js";
import {isImportMetaResolve, parseImports} from "./javascript/imports.js";
import {parseProgram} from "./javascript/parse.js";
Expand Down Expand Up @@ -64,7 +65,7 @@ export function rewriteNpmImports(input: string, resolve: (s: string) => string
const value = getStringLiteralValue(source);
const resolved = resolve(value);
if (resolved === undefined || value === resolved) return;
output.replaceLeft(source.start, source.end, JSON.stringify(resolved));
output.replaceLeft(source.start, source.end, annotate(resolved));
}

// TODO Preserve the source map, but download it too.
Expand Down
3 changes: 2 additions & 1 deletion src/rollup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {AstNode, OutputChunk, Plugin, ResolveIdResult} from "rollup";
import {rollup} from "rollup";
import esbuild from "rollup-plugin-esbuild";
import {getClientPath, getStylePath} from "./files.js";
import annotate from "./javascript/annotate.js";
import type {StringLiteral} from "./javascript/source.js";
import {getStringLiteralValue, isStringLiteral} from "./javascript/source.js";
import {resolveNpmImport} from "./npm.js";
Expand Down Expand Up @@ -177,7 +178,7 @@ function importMetaResolve(path: string, resolveImport: ImportResolver): Plugin
for (const source of resolves) {
const specifier = getStringLiteralValue(source);
const resolution = await resolveImport(specifier);
if (resolution) output.replaceLeft(source.start, source.end, JSON.stringify(relativePath(path, resolution)));
if (resolution) output.replaceLeft(source.start, source.end, annotate(relativePath(path, resolution)));
}

return {code: String(output)};
Expand Down
63 changes: 63 additions & 0 deletions test/javascript/annotate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* This file is not suffixed with '-test'; it expects to run with an extra
* OBSERVABLE_ANNOTATE_FILES=true environment variable.
*/
import assert from "node:assert";
import type {TranspileModuleOptions} from "../../src/javascript/transpile.js";
import {transpileModule} from "../../src/javascript/transpile.js";
import {fromJsDelivrPath, rewriteNpmImports} from "../../src/npm.js";
import {relativePath} from "../../src/path.js";

// prettier-ignore
describe("annotates", () => {
const options: TranspileModuleOptions = {root: "src", path: "test.js"};
it("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"/* observablehq-file */;');
});
it("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"/* observablehq-file */;');
});
it("dynamic 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"/* observablehq-file */);');
});
it("/npm/ exports", () => {
assert.strictEqual(rewriteNpmImports('export * from "/npm/[email protected]/dist/d3-array.js";\n', (v) => resolve("/_npm/[email protected]/dist/d3.js", v)), 'export * from "../../[email protected]/dist/d3-array.js"/* observablehq-file */;\n');
});
it("/npm/ imports", () => {
assert.strictEqual(rewriteNpmImports('import "/npm/[email protected]/dist/d3-array.js";\n', (v) => resolve("/_npm/[email protected]/dist/d3.js", v)), 'import "../../[email protected]/dist/d3-array.js"/* observablehq-file */;\n');
assert.strictEqual(rewriteNpmImports('import "/npm/[email protected]/dist/d3-array.js";\n', (v) => resolve("/_npm/[email protected]/d3.js", v)), 'import "../[email protected]/dist/d3-array.js"/* observablehq-file */;\n');
});
it("named imports", () => {
assert.strictEqual(rewriteNpmImports('import {sort} from "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import {sort} from "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("empty imports", () => {
assert.strictEqual(rewriteNpmImports('import "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("default imports", () => {
assert.strictEqual(rewriteNpmImports('import d3 from "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import d3 from "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("namespace imports", () => {
assert.strictEqual(rewriteNpmImports('import * as d3 from "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import * as d3 from "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("named exports", () => {
assert.strictEqual(rewriteNpmImports('export {sort} from "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'export {sort} from "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("namespace exports", () => {
assert.strictEqual(rewriteNpmImports('export * from "/npm/[email protected]/+esm";\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'export * from "../[email protected]/_esm.js"/* observablehq-file */;\n');
});
it("dynamic imports with static module specifiers", () => {
assert.strictEqual(rewriteNpmImports('import("/npm/[email protected]/+esm");\n', (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import("../[email protected]/_esm.js"/* observablehq-file */);\n');
assert.strictEqual(rewriteNpmImports("import(`/npm/[email protected]/+esm`);\n", (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import("../[email protected]/_esm.js"/* observablehq-file */);\n');
assert.strictEqual(rewriteNpmImports("import('/npm/[email protected]/+esm');\n", (v) => resolve("/_npm/[email protected]/_esm.js", v)), 'import("../[email protected]/_esm.js"/* observablehq-file */);\n');
});
});

function resolve(path: string, specifier: string): string {
return specifier.startsWith("/npm/") ? relativePath(path, fromJsDelivrPath(specifier)) : specifier;
}