Skip to content

style nits #82

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

style nits #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 6 additions & 12 deletions src/preview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import type {ParseResult} from "./markdown.js";
import {diffMarkdown, readMarkdown} from "./markdown.js";
import {readPages} from "./navigation.js";
import {renderPreview} from "./render.js";
import type {CellResolver} from "./resolver.js";
import {makeCLIResolver} from "./resolver.js";
import {makeCLIResolver, type CellResolver} from "./resolver.js";

const publicRoot = join(dirname(fileURLToPath(import.meta.url)), "..", "public");

Expand Down Expand Up @@ -136,16 +135,11 @@ class FileWatchers {
}

function resolveDiffs(diff: ReturnType<typeof diffMarkdown>, resolver: CellResolver): ReturnType<typeof diffMarkdown> {
for (const item of diff) {
if (item.type === "add") {
for (const addItem of item.items) {
if (addItem.type === "cell" && "databases" in addItem) {
Object.assign(addItem, resolver(addItem));
}
}
}
}
return diff;
return diff.map((item) =>
item.type === "add"
? {...item, items: item.items.map((addItem) => (addItem.type === "cell" ? resolver(addItem) : addItem))}
: item
);
Copy link
Member Author

Choose a reason for hiding this comment

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

Here I’m just trying to avoid in-place mutation. It doesn’t really matter since this is only used internally, but since the CellResolver API does a copy-on-write, it feels reasonable to do that here. And it avoids needing to leave a comment warning that this method does in-place mutation.

}

function handleWatch(socket: WebSocket, options: {root: string; resolver: CellResolver}) {
Expand Down
13 changes: 5 additions & 8 deletions src/render.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {computeHash} from "./hash.js";
import {type FileReference, type ImportReference} from "./javascript.js";
import type {CellPiece} from "./markdown.js";
import {parseMarkdown, type ParseResult} from "./markdown.js";
import {parseMarkdown, type CellPiece, type ParseResult} from "./markdown.js";

export interface Render {
html: string;
Expand Down Expand Up @@ -63,14 +62,12 @@ ${
${JSON.stringify({imports: Object.fromEntries(Array.from(imports, ([name, href]) => [name, href]))}, null, 2)}
</script>
${Array.from(imports.values())
.concat(parseResult.imports.filter(({name}) => name.startsWith("./")).map(({name}) => `/_file/${name.slice(2)}`))
.concat(
parseResult.imports.filter(({name}) => name.startsWith("./")).map(({name}) => `/_file/${name.slice(2)}`),
parseResult.cells.some((cell) => cell.databases?.length) ? "/_observablehq/database.js" : []
)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is, again, a trivial nit pick, but this approach avoids a blank line being emitted when the page doesn’t use a database.

.map((href) => `<link rel="modulepreload" href="${href}">`)
.join("\n")}
${
parseResult.cells.some((cell) => cell.databases?.length)
? `<link rel="modulepreload" href="/_observablehq/database.js">`
: ""
}
<script type="module">

import {${preview ? "open, " : ""}define} from "/_observablehq/client.js";
Expand Down
13 changes: 6 additions & 7 deletions src/resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {homedir} from "os";
import {join} from "path";
import {createHmac} from "node:crypto";
import {readFile} from "node:fs/promises";
import type {CellPiece} from "./markdown.js";
import {homedir} from "node:os";
import {join} from "node:path";
import {type CellPiece} from "./markdown.js";

export type CellResolver = (cell: CellPiece) => CellPiece;

Expand Down Expand Up @@ -41,7 +41,7 @@ export async function readDatabaseProxyConfig(): Promise<DatabaseProxyConfig | n
let observableConfig;
try {
observableConfig = JSON.parse(await readFile(configFile, "utf-8")) as ObservableConfig | null;
} catch (error) {
} catch {
// Ignore missing config file
}
return observableConfig && observableConfig[key];
Expand All @@ -65,13 +65,13 @@ function decodeSecret(secret: string): Record<string, string> {
function encodeToken(payload: {name: string}, secret): string {
const data = JSON.stringify(payload);
const hmac = createHmac("sha256", Buffer.from(secret, "hex")).update(data).digest();
return `${Buffer.from(data).toString("base64") + "." + Buffer.from(hmac).toString("base64")}`;
return `${Buffer.from(data).toString("base64")}.${Buffer.from(hmac).toString("base64")}`;
}

export async function makeCLIResolver(): Promise<CellResolver> {
const config = await readDatabaseProxyConfig();
return (cell: CellPiece): CellPiece => {
if ("databases" in cell && cell.databases !== undefined) {
if (cell.databases !== undefined) {
cell = {
...cell,
databases: cell.databases.map((ref) => {
Expand All @@ -81,7 +81,6 @@ export async function makeCLIResolver(): Promise<CellResolver> {
url.protocol = db.ssl !== "disabled" ? "https:" : "http:";
url.host = db.host;
url.port = String(db.port);
url.toString();
return {
...ref,
token: encodeToken(ref, db.secret),
Expand Down