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

Automatically annotate file paths if dynamicPaths include JS modules or is a function #1815

Closed
wants to merge 2 commits into from
Closed
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
Next Next commit
Automatically annotate file paths if dynamicPaths include JS modules …
…or is a function
mythmon committed Nov 13, 2024
commit accd344f17622481394d3e1e161f7a52976d866d
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -24,12 +24,10 @@
"docs:deploy": "tsx --no-warnings=ExperimentalWarning ./src/bin/observable.ts deploy",
"build": "rimraf dist && node build.js --outdir=dist --outbase=src \"src/**/*.{ts,js,css}\" --ignore \"**/*.d.ts\"",
"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:all",
"test:coverage": "c8 --check-coverage --lines 80 --per-file yarn test:mocha",
"test:build": "rimraf test/build && rimraf --glob test/.observablehq/cache test/input/build/*/.observablehq/cache && 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:mocha:serial -p",
"test:mocha:serial": "yarn test:build && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/*-test.js\"",
"test:mocha:annotate": "yarn test:build && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 OBSERVABLE_ANNOTATE_FILES=true TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/annotate.js\"",
"test:mocha:all": "yarn test:mocha && cross-env OBSERVABLE_TELEMETRY_DISABLE=1 OBSERVABLE_ANNOTATE_FILES=true TZ=America/Los_Angeles mocha --timeout 30000 \"test/build/test/**/annotate.js\"",
"test:lint": "eslint src test --max-warnings=0",
"test:prettier": "prettier --check src test",
"test:tsc": "tsc --noEmit",
14 changes: 13 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
@@ -191,9 +191,21 @@ async function importConfig(path: string): Promise<ConfigSpec> {
return (await import(`${pathToFileURL(path).href}?${mtimeMs}`)).default;
}

export async function readConfig(configPath?: string, root?: string): Promise<Config> {
export async function readConfig(configPath?: string, root?: string, normalize?: never | true): Promise<Config>;
export async function readConfig(
configPath: string | undefined,
root: string | undefined,
normalize: false
): Promise<ConfigSpec>;
export async function readConfig(
configPath?: string,
root?: string,
normalize: boolean = true
): Promise<Config | ConfigSpec> {
if (configPath === undefined) configPath = await resolveDefaultConfig(root);
if (configPath === undefined) return normalizeConfig(undefined, root);
const unnormalized = await importConfig(configPath);
if (!normalize) return unnormalized;
return normalizeConfig(await importConfig(configPath), root, configPath);
}

26 changes: 21 additions & 5 deletions src/javascript/annotate.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,25 @@
import {readConfig} from "../config.js";
import {isPathImport} from "../path.js";

const annotate = process.env["OBSERVABLE_ANNOTATE_FILES"];
if (annotate && annotate !== "true") throw new Error(`unsupported OBSERVABLE_ANNOTATE_FILES: ${annotate}`);
const unnormalizedConfig = await readConfig(undefined, undefined, false);
const configShouldAnnotate = Array.isArray(unnormalizedConfig.dynamicPaths)
? unnormalizedConfig.dynamicPaths.some((path) => path.endsWith(".js"))
: typeof unnormalizedConfig.dynamicPaths === "function";

/** Annotate a path to a local import or file so it can be reworked server-side. */
export const annotatePath = annotate
? (uri: string) => `${JSON.stringify(uri)}${isPathImport(uri) ? "/* observablehq-file */" : ""}`
: JSON.stringify;
export function annotatePath(uri: string) {
const envShouldAnnotate =
process.env["OBSERVABLE_ANNOTATE_FILES"] === "true"
? true
: process.env["OBSERVABLE_ANNOTATE_FILES"] === "false"
? false
: process.env["OBSERVABLE_ANNOTATE_FILES"];
if (envShouldAnnotate !== true && envShouldAnnotate !== false && envShouldAnnotate !== undefined) {
throw new Error(`unsupported OBSERVABLE_ANNOTATE_FILES: ${envShouldAnnotate}`);
}

const shouldAnnotate = envShouldAnnotate ?? configShouldAnnotate;
return shouldAnnotate
? `${JSON.stringify(uri)}${isPathImport(uri) ? "/* observablehq-file */" : ""}`
: JSON.stringify(uri);
}
2 changes: 2 additions & 0 deletions test/build-test.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import {ascending, difference} from "d3-array";
import type {BuildManifest} from "../src/build.js";
import {FileBuildEffects, build} from "../src/build.js";
import {normalizeConfig, readConfig, setCurrentDate} from "../src/config.js";
import {mockAnnotateFileEnv} from "./javascript/annotate-test.js";
import {mockDuckDB} from "./mocks/duckdb.js";
import {mockJsDelivr} from "./mocks/jsdelivr.js";
import {mockJsr} from "./mocks/jsr.js";
@@ -35,6 +36,7 @@ describe("build", () => {
mockJsDelivr();
mockJsr();
mockDuckDB();
mockAnnotateFileEnv(false);

// Each sub-directory of test/input/build is a test case.
const inputRoot = "test/input/build";
13 changes: 12 additions & 1 deletion test/javascript/annotate.ts → test/javascript/annotate-test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
// 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";

export function mockAnnotateFileEnv(value = true) {
let annotateFileEnvBefore: string | undefined;
before(() => {
annotateFileEnvBefore = process.env["OBSERVABLE_ANNOTATE_FILES"];
process.env["OBSERVABLE_ANNOTATE_FILES"] = JSON.stringify(value);
});
after(() => {
process.env["OBSERVABLE_ANNOTATE_FILES"] = annotateFileEnvBefore;
});
}

// prettier-ignore
describe("annotates", () => {
mockAnnotateFileEnv(true);
const options: TranspileModuleOptions = {root: "src", path: "test.js"};
it("npm imports", async () => {
const input = 'import "npm:d3-array";';
4 changes: 4 additions & 0 deletions test/javascript/transpile-test.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ import type {TranspileModuleOptions} from "../../src/javascript/transpile.js";
import {transpileJavaScript, transpileModule} from "../../src/javascript/transpile.js";
import {isPathImport} from "../../src/path.js";
import {mockJsDelivr} from "../mocks/jsdelivr.js";
import {mockAnnotateFileEnv} from "./annotate-test.js";

function isJsFile(inputRoot: string, fileName: string) {
if (!fileName.endsWith(".js")) return false;
@@ -86,6 +87,7 @@ function runTests(inputRoot: string, outputRoot: string, filter: (name: string)
}

describe("transpileJavaScript(input, options)", () => {
mockAnnotateFileEnv(false);
runTests("test/input", "test/output");
runTests("test/input/imports", "test/output/imports", (name) => name.endsWith("-import.js"));
it("trims leading and trailing newlines", async () => {
@@ -102,6 +104,7 @@ async function testFile(target: string, path: string): Promise<string> {
}

describe("transpileModule(input, root, path, sourcePath)", () => {
mockAnnotateFileEnv(false);
it("rewrites relative files with import.meta.resolve", async () => {
assert.strictEqual(await testFile("./test.txt", "test.js"), 'FileAttachment("../test.txt", import.meta.url)'); // prettier-ignore
assert.strictEqual(await testFile("./sub/test.txt", "test.js"), 'FileAttachment("../sub/test.txt", import.meta.url)'); // prettier-ignore
@@ -122,6 +125,7 @@ describe("transpileModule(input, root, path, sourcePath)", () => {

describe("transpileModule(input, root, path)", () => {
mockJsDelivr();
mockAnnotateFileEnv(false);
const options: TranspileModuleOptions = {root: "src", path: "test.js"};
it("rewrites relative files with import.meta.resolve", async () => {
assert.strictEqual(await testFile("./test.txt", "test.js"), 'FileAttachment("../test.txt", import.meta.url)'); // prettier-ignore
8 changes: 8 additions & 0 deletions test/npm-test.ts
Original file line number Diff line number Diff line change
@@ -4,10 +4,12 @@ import {join} from "node:path/posix";
import {extractNpmSpecifier, initializeNpmVersionCache, parseNpmSpecifier} from "../src/npm.js";
import {fromJsDelivrPath, getDependencyResolver, resolveNpmImport, rewriteNpmImports} from "../src/npm.js";
import {relativePath} from "../src/path.js";
import {mockAnnotateFileEnv} from "./javascript/annotate-test.js";
import {mockJsDelivr} from "./mocks/jsdelivr.js";

describe("getDependencyResolver(root, path, input)", () => {
mockJsDelivr();
mockAnnotateFileEnv(false);
it("finds /npm/ imports and re-resolves their versions", async () => {
const root = "test/input/build/simple-public";
const specifier = "/npm/d3-array@3.2.3/dist/d3-array.js";
@@ -23,6 +25,7 @@ describe("getDependencyResolver(root, path, input)", () => {
});

describe("parseNpmSpecifier(specifier)", () => {
mockAnnotateFileEnv(false);
it("parses the name", () => {
assert.deepStrictEqual(parseNpmSpecifier("d3-array"), {name: "d3-array", range: undefined, path: undefined});
});
@@ -50,6 +53,7 @@ describe("parseNpmSpecifier(specifier)", () => {

describe("resolveNpmImport(root, specifier)", () => {
mockJsDelivr();
mockAnnotateFileEnv(false);
const root = "test/input/build/simple";
it("implicitly adds ._esm.js for specifiers without an extension", async () => {
assert.strictEqual(await resolveNpmImport(root, "d3-array"), "/_npm/d3-array@3.2.4/_esm.js");
@@ -75,6 +79,7 @@ describe("resolveNpmImport(root, specifier)", () => {
});

describe("extractNpmSpecifier(path)", () => {
mockAnnotateFileEnv(false);
it("returns the npm specifier for the given local npm path", () => {
assert.strictEqual(extractNpmSpecifier("/_npm/d3@7.8.5/_esm.js"), "d3@7.8.5/+esm");
assert.strictEqual(extractNpmSpecifier("/_npm/d3@7.8.5/dist/d3.js"), "d3@7.8.5/dist/d3.js");
@@ -89,6 +94,7 @@ describe("extractNpmSpecifier(path)", () => {
});

describe("fromJsDelivrPath(path)", () => {
mockAnnotateFileEnv(false);
it("returns the local npm path for the given jsDelivr path", () => {
assert.strictEqual(fromJsDelivrPath("/npm/d3@7.8.5/+esm"), "/_npm/d3@7.8.5/_esm.js");
assert.strictEqual(fromJsDelivrPath("/npm/d3@7.8.5/dist/d3.js"), "/_npm/d3@7.8.5/dist/d3.js");
@@ -102,6 +108,7 @@ describe("fromJsDelivrPath(path)", () => {

// prettier-ignore
describe("rewriteNpmImports(input, resolve)", () => {
mockAnnotateFileEnv(false);
it("rewrites /npm/ imports to /_npm/", () => {
assert.strictEqual(rewriteNpmImports('export * from "/npm/d3-array@3.2.4/dist/d3-array.js";\n', (v) => resolve("/_npm/d3@7.8.5/dist/d3.js", v)), 'export * from "../../d3-array@3.2.4/dist/d3-array.js";\n');
});
@@ -148,6 +155,7 @@ describe("rewriteNpmImports(input, resolve)", () => {
});

describe("initializeNpmVersionCache(root, dir)", () => {
mockAnnotateFileEnv(false);
const root = join("test", "input", "npm");
const dir = join(root, ".observablehq", "cache", "_npm");
before(async () => {