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

getModuleHash needs to consider data loaders #1662

Merged
merged 9 commits into from
Sep 17, 2024
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
12 changes: 6 additions & 6 deletions src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import {basename, dirname, extname, join} from "node:path/posix";
import type {Config} from "./config.js";
import {CliError} from "./error.js";
import {getClientPath, prepareOutput} from "./files.js";
import {findModule, getLocalModuleHash, getModuleHash, readJavaScript} from "./javascript/module.js";
import {findModule, getModuleHash, readJavaScript} from "./javascript/module.js";
import {transpileModule} from "./javascript/transpile.js";
import type {Logger, Writer} from "./logger.js";
import type {MarkdownPage} from "./markdown.js";
import {populateNpmCache, resolveNpmImport, rewriteNpmImports} from "./npm.js";
import {isAssetPath, isPathImport, relativePath, resolvePath, within} from "./path.js";
import {renderModule, renderPage} from "./render.js";
import type {Resolvers} from "./resolvers.js";
import {getModuleResolver, getModuleResolvers, getResolvers} from "./resolvers.js";
import {resolveImportPath, resolveStylesheetPath} from "./resolvers.js";
import {getModuleResolvers, getResolvers} from "./resolvers.js";
import {resolveStylesheetPath} from "./resolvers.js";
import {bundleStyles, rollupClient} from "./rollup.js";
import {searchIndex} from "./search.js";
import {Telemetry} from "./telemetry.js";
Expand Down Expand Up @@ -230,7 +230,7 @@ export async function build(
// string. Note that this hash is not of the content of the module itself, but
// of the transitive closure of the module and its imports and files.
const resolveLocalImport = async (path: string): Promise<string> => {
const hash = (await getLocalModuleHash(root, path)).slice(0, 8);
const hash = (await loaders.getLocalModuleHash(path)).slice(0, 8);
return applyHash(join("/_import", path), hash);
};
for (const path of localImports) {
Expand All @@ -239,7 +239,7 @@ export async function build(
const sourcePath = join(root, module.path);
const importPath = join("_import", module.path);
effects.output.write(`${faint("copy")} ${sourcePath} ${faint("→")} `);
const resolveImport = getModuleResolver(root, path);
const resolveImport = loaders.getModuleResolver(path);
const input = await readJavaScript(sourcePath);
const contents = await transpileModule(input, {
root,
Expand Down Expand Up @@ -267,7 +267,7 @@ export async function build(
}
});
const alias = await resolveLocalImport(path);
aliases.set(resolveImportPath(root, path), alias);
aliases.set(loaders.resolveImportPath(path), alias);
await effects.writeFile(alias, contents);
}

Expand Down
18 changes: 7 additions & 11 deletions src/javascript/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ const moduleInfoCache = new Map<string, ModuleInfo>();
* has invalid syntax, returns the hash of empty content; likewise ignores any
* transitive imports or files that are invalid or do not exist.
*/
export function getModuleHash(root: string, path: string): string {
return getModuleHashInternal(root, path).digest("hex");
export function getModuleHash(root: string, path: string, getHash?: (p: string) => string): string {
return getModuleHashInternal(root, path, getHash).digest("hex");
}

function getModuleHashInternal(root: string, path: string): Hash {
function getModuleHashInternal(root: string, path: string, getHash = (p: string) => getFileHash(root, p)): Hash {
const hash = createHash("sha256");
const paths = new Set([path]);
for (const path of paths) {
Expand All @@ -73,14 +73,10 @@ function getModuleHashInternal(root: string, path: string): Hash {
paths.add(resolvePath(path, i));
}
for (const i of info.files) {
const f = getFileInfo(root, resolvePath(path, i));
if (!f) continue; // ignore missing file
hash.update(f.hash);
hash.update(getHash(resolvePath(path, i)));
}
} else {
const info = getFileInfo(root, path); // e.g., import.meta.resolve("foo.json")
if (!info) continue; // ignore missing file
hash.update(info.hash);
hash.update(getHash(path)); // e.g., import.meta.resolve("foo.json")
}
}
return hash;
Expand All @@ -92,8 +88,8 @@ function getModuleHashInternal(root: string, path: string): Hash {
* during build because we want the hash of the built module to change if the
* version of an imported npm package changes.
*/
export async function getLocalModuleHash(root: string, path: string): Promise<string> {
const hash = getModuleHashInternal(root, path);
export async function getLocalModuleHash(root: string, path: string, getHash?: (p: string) => string): Promise<string> {
const hash = getModuleHashInternal(root, path, getHash);
const info = getModuleInfo(root, path);
if (info) {
const globalPaths = new Set<string>();
Expand Down
25 changes: 24 additions & 1 deletion src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import {maybeStat, prepareOutput, visitFiles} from "./files.js";
import {FileWatchers} from "./fileWatchers.js";
import {formatByteSize} from "./format.js";
import type {FileInfo} from "./javascript/module.js";
import {findModule, getFileInfo} from "./javascript/module.js";
import {findModule, getFileInfo, getLocalModuleHash, getModuleHash} from "./javascript/module.js";
import type {Logger, Writer} from "./logger.js";
import type {MarkdownPage, ParseOptions} from "./markdown.js";
import {parseMarkdown} from "./markdown.js";
import {getModuleResolver, resolveImportPath} from "./resolvers.js";
import type {Params} from "./route.js";
import {isParameterized, requote, route} from "./route.js";
import {cyan, faint, green, red, yellow} from "./tty.js";
Expand Down Expand Up @@ -306,6 +307,12 @@ export class LoaderResolver {
return path === name ? hash : createHash("sha256").update(hash).update(String(info.mtimeMs)).digest("hex");
}

getOutputFileHash(name: string): string {
const info = this.getOutputInfo(name);
if (!info) throw new Error(`output file not found: ${name}`);
return info.hash;
}

getSourceInfo(name: string): FileInfo | undefined {
return getFileInfo(this.root, this.getSourceFilePath(name));
}
Expand All @@ -314,6 +321,22 @@ export class LoaderResolver {
return getFileInfo(this.root, this.getOutputFilePath(name));
}

getLocalModuleHash(path: string): Promise<string> {
return getLocalModuleHash(this.root, path, (p) => this.getOutputFileHash(p));
}

getModuleHash(path: string): string {
return getModuleHash(this.root, path, (p) => this.getSourceFileHash(p));
}

getModuleResolver(path: string, servePath?: string): (specifier: string) => Promise<string> {
return getModuleResolver(this.root, path, servePath, (p) => this.getSourceFileHash(p));
}

resolveImportPath(path: string): string {
return resolveImportPath(this.root, path, (p) => this.getSourceFileHash(p));
}

resolveFilePath(path: string): string {
return `/${join("_file", path)}?sha=${this.getSourceFileHash(path)}`;
}
Expand Down
1 change: 1 addition & 0 deletions src/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export class PreviewServer {
root,
path,
params: module.params,
resolveImport: loaders.getModuleResolver(path),
resolveFile: (name) => loaders.resolveFilePath(resolvePath(path, name)),
resolveFileInfo: (name) => loaders.getSourceInfo(resolvePath(path, name))
});
Expand Down
15 changes: 8 additions & 7 deletions src/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const builtins = new Map<string, string>([
* them to any files referenced by static HTML.
*/
export async function getResolvers(page: MarkdownPage, config: ResolversConfig): Promise<Resolvers> {
const {root, path, globalStylesheets: defaultStylesheets, loaders} = config;
const {path, globalStylesheets: defaultStylesheets, loaders} = config;
const hash = createHash("sha256").update(page.body).update(JSON.stringify(page.data));
const assets = new Set<string>();
const files = new Set<string>();
Expand Down Expand Up @@ -131,7 +131,7 @@ export async function getResolvers(page: MarkdownPage, config: ResolversConfig):
// Compute the content hash.
for (const f of assets) hash.update(loaders.getSourceFileHash(resolvePath(path, f)));
for (const f of files) hash.update(loaders.getSourceFileHash(resolvePath(path, f)));
for (const i of localImports) hash.update(getModuleHash(root, resolvePath(path, i)));
for (const i of localImports) hash.update(loaders.getModuleHash(resolvePath(path, i)));
if (page.style && isPathImport(page.style)) hash.update(loaders.getSourceFileHash(resolvePath(path, page.style)));

// Add implicit imports for standard library built-ins, such as d3 and Plot.
Expand Down Expand Up @@ -332,7 +332,7 @@ async function resolveResolvers(

function resolveImport(specifier: string): string {
return isPathImport(specifier)
? relativePath(path, resolveImportPath(root, resolvePath(path, specifier)))
? relativePath(path, loaders.resolveImportPath(resolvePath(path, specifier)))
: builtins.has(specifier)
? relativePath(path, builtins.get(specifier)!)
: specifier.startsWith("observablehq:")
Expand Down Expand Up @@ -435,11 +435,12 @@ export async function getModuleStaticImports(root: string, path: string): Promis
export function getModuleResolver(
root: string,
path: string,
servePath = `/${join("_import", path)}`
servePath = `/${join("_import", path)}`,
getHash?: (path: string) => string
): (specifier: string) => Promise<string> {
return async (specifier) => {
return isPathImport(specifier)
? relativePath(servePath, resolveImportPath(root, resolvePath(path, specifier)))
? relativePath(servePath, resolveImportPath(root, resolvePath(path, specifier), getHash))
: builtins.has(specifier)
? relativePath(servePath, builtins.get(specifier)!)
: specifier.startsWith("observablehq:")
Expand All @@ -456,8 +457,8 @@ export function resolveStylesheetPath(root: string, path: string): string {
return `/${join("_import", path)}?sha=${getFileHash(root, path)}`;
}

export function resolveImportPath(root: string, path: string): string {
return `/${join("_import", path)}?sha=${getModuleHash(root, path)}`;
export function resolveImportPath(root: string, path: string, getHash?: (name: string) => string): string {
return `/${join("_import", path)}?sha=${getModuleHash(root, path, getHash)}`;
}

// Returns any inputs that are not declared in outputs. These typically refer to
Expand Down
1 change: 1 addition & 0 deletions test/input/build/data-loaders/import-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {test} from "./test.js";
7 changes: 7 additions & 0 deletions test/input/build/data-loaders/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Data loader test

```js
import {test} from "./import-test.js";

display(await test);
```
3 changes: 3 additions & 0 deletions test/input/build/data-loaders/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import {FileAttachment} from "npm:@observablehq/stdlib";

export const test = FileAttachment("./test.txt").text();
1 change: 1 addition & 0 deletions test/input/build/data-loaders/test.txt.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.stdout.write("test\n");
1 change: 1 addition & 0 deletions test/output/build/data-loaders/_file/test.f2ca1bb6.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {test} from "./test.513ced9f.js";
3 changes: 3 additions & 0 deletions test/output/build/data-loaders/_import/test.513ced9f.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import {FileAttachment} from "../_observablehq/stdlib.00000003.js";

export const test = FileAttachment({"name":"../test.txt","mimeType":"text/plain","path":"../_file/test.f2ca1bb6.txt","lastModified":/* ts */1706742000000,"size":5}, import.meta.url).text();
Empty file.
Empty file.
Empty file.
43 changes: 43 additions & 0 deletions test/output/build/data-loaders/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!DOCTYPE html>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
<meta name="generator" content="Observable Framework v1.0.0-test">
<title>Data loader test</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link rel="preload" as="style" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
<link rel="preload" as="style" href="./_observablehq/theme-air,near-midnight.00000004.css">
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
<link rel="stylesheet" type="text/css" href="./_observablehq/theme-air,near-midnight.00000004.css">
<link rel="modulepreload" href="./_observablehq/client.00000001.js">
<link rel="modulepreload" href="./_observablehq/runtime.00000002.js">
<link rel="modulepreload" href="./_observablehq/stdlib.00000003.js">
<link rel="modulepreload" href="./_import/import-test.7d216225.js">
<link rel="modulepreload" href="./_import/test.513ced9f.js">
<script type="module">

import {define} from "./_observablehq/client.00000001.js";
import {registerFile} from "./_observablehq/stdlib.00000003.js";

registerFile("./test.txt", {"name":"./test.txt","mimeType":"text/plain","path":"./_file/test.f2ca1bb6.txt","lastModified":/* ts */1706742000000,"size":5});

define({id: "05e74070", inputs: ["display"], outputs: ["test"], body: async (display) => {
const {test} = await import("./_import/import-test.7d216225.js");

display(await test);
return {test};
}});

</script>
<aside id="observablehq-toc" data-selector="h1:not(:first-of-type)[id], h2:first-child[id], :not(h1) + h2[id]">
<nav>
</nav>
</aside>
<div id="observablehq-center">
<main id="observablehq-main" class="observablehq">
<h1 id="data-loader-test" tabindex="-1"><a class="observablehq-header-anchor" href="#data-loader-test">Data loader test</a></h1>
<div class="observablehq observablehq--block"><!--:05e74070:--></div>
</main>
<footer id="observablehq-footer">
<div>Built with <a href="https://observablehq.com/" target="_blank" rel="noopener noreferrer">Observable</a> on <a title="2024-01-10T16:00:00">Jan 10, 2024</a>.</div>
</footer>
</div>