diff --git a/web/src/lib/components/repo/pulls/PullReviewComment.svelte b/web/src/lib/components/repo/pulls/PullReviewComment.svelte index 34f5f8c38..7d9ef9a40 100644 --- a/web/src/lib/components/repo/pulls/PullReviewComment.svelte +++ b/web/src/lib/components/repo/pulls/PullReviewComment.svelte @@ -9,7 +9,7 @@ import User from "$lib/components/ui/User.svelte"; import type { MarkupContext } from "$lib/markup"; import { enhanceCodeblocks } from "$lib/markup/highlight"; - import { renderMarkdown } from "$lib/markup/markdown"; + import { renderMarkup } from "$lib/markup"; import PullReviewCommentEditor from "$lib/components/repo/pulls/PullReviewCommentEditor.svelte"; interface Props { @@ -33,7 +33,7 @@ }: Props = $props(); async function submit(body: string) { - const bodyHtml = await renderMarkdown(body, markup); + const bodyHtml = await renderMarkup(body, markup); await onedit({ ...comment, body, bodyHtml }); editing = false; } diff --git a/web/src/lib/markup/render.test.ts b/web/src/lib/markup/render.test.ts new file mode 100644 index 000000000..966d0cde9 --- /dev/null +++ b/web/src/lib/markup/render.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import { renderDocument, renderMarkup } from "$lib/markup/render"; +import type { MarkupContext } from "$lib/markup/paths"; + +const ctx: MarkupContext = { repo: "ada.test/infra", ref: "main", host: "tangled.org" }; + +const keyOf = (html: string | null): string => { + const key = /^
/.exec(html ?? "")?.[1]; + if (!key) throw new Error(`no markup key in ${html}`); + return key; +}; + +describe("renderDocument", () => { + it("wraps what it renders under a key", async () => { + const html = await renderDocument("README.md", "# hello", ctx); + expect(keyOf(html)).toMatch(/^[0-9a-z]+\.[0-9a-z]+\.[0-9a-z]+$/); + expect(html).toContain(" { + expect(await renderDocument("notes.txt", "# hello", ctx)).toBeNull(); + }); + + it("keys the same contents rendered elsewhere apart", async () => { + const here = keyOf(await renderDocument("README.md", "see [x](y)", ctx)); + const elsewhere = keyOf( + await renderDocument("README.md", "see [x](y)", { ...ctx, ref: "dev" }) + ); + const copied = keyOf(await renderDocument("docs/README.md", "see [x](y)", ctx)); + expect(elsewhere).not.toBe(here); + expect(copied).not.toBe(here); + }); + + it("keys different contents apart", async () => { + expect(keyOf(await renderDocument("README.md", "# a", ctx))).not.toBe( + keyOf(await renderDocument("README.md", "# b", ctx)) + ); + }); + + it("gives the same contents the same key every time", async () => { + expect(keyOf(await renderDocument("README.md", "# a", ctx))).toBe( + keyOf(await renderDocument("README.md", "# a", ctx)) + ); + }); +}); + +describe("renderMarkup", () => { + // a body and a file with the same contents need different keys + it("wraps a body under a key of its own", async () => { + const html = await renderMarkup("a **body**", ctx); + expect(html).toContain("body"); + expect(keyOf(html)).not.toBe(keyOf(await renderDocument("README.md", "a **body**", ctx))); + }); +}); diff --git a/web/src/lib/markup/render.ts b/web/src/lib/markup/render.ts index 38f8d7a51..22fcad805 100644 --- a/web/src/lib/markup/render.ts +++ b/web/src/lib/markup/render.ts @@ -1,24 +1,58 @@ +import { browser } from "$app/environment"; import { isMarkdownFile } from "$lib/markup/format"; import type { MarkupContext } from "$lib/markup/paths"; -export const renderDocument = async ( - filename: string, +// hydration re-runs every load, so the browser hands back the server's render +// instead of loading the markdown pipeline again + +const MARKUP_KEY = "data-markup-key"; + +// the key is only compared against markup already on this page, so stable and +// short is enough, not cryptographic +const digest = (value: string): string => { + let hash = 5381; + for (let i = 0; i < value.length; i += 1) hash = (hash * 33) ^ value.charCodeAt(i); + return (hash >>> 0).toString(36); +}; + +const keyOf = (filename: string, contents: string, ctx: MarkupContext): string => + [ + digest([ctx.host ?? "", ctx.repo, ctx.ref, ctx.dir ?? "", filename].join("\n")), + contents.length.toString(36), + digest(contents) + ].join("."); + +const mark = (key: string, html: string): string => `
${html}
`; + +// reads back a render this module wrote for the same contents and context +const rendered = (key: string): string | null => { + if (!browser) return null; + const node = document.querySelector(`[${MARKUP_KEY}="${key}"]`); + return node instanceof HTMLElement ? node.innerHTML : null; +}; + +const render = async ( + key: string, contents: string, ctx: MarkupContext ): Promise => { - if (!isMarkdownFile(filename)) return null; + const reused = rendered(key); + if (reused !== null) return mark(key, reused); + const { SOURCE_LIMIT, renderMarkdown } = await import("./markdown"); if (contents.length > SOURCE_LIMIT) return null; - return renderMarkdown(contents, ctx); + const html = await renderMarkdown(contents, ctx); + return html === null ? null : mark(key, html); }; -// like renderDocument, but for content that is always markdown (issue and -// comment bodies) rather than a repo file with an extension to sniff -export const renderMarkup = async ( +export const renderDocument = async ( + filename: string, contents: string, ctx: MarkupContext -): Promise => { - const { SOURCE_LIMIT, renderMarkdown } = await import("./markdown"); - if (contents.length > SOURCE_LIMIT) return null; - return renderMarkdown(contents, ctx); -}; +): Promise => + isMarkdownFile(filename) ? render(keyOf(filename, contents, ctx), contents, ctx) : null; + +// like renderDocument, but for content that is always markdown (issue and +// comment bodies) rather than a repo file with an extension to sniff +export const renderMarkup = (contents: string, ctx: MarkupContext): Promise => + render(keyOf("", contents, ctx), contents, ctx); diff --git a/web/src/markup.css b/web/src/markup.css index 9594249f2..d8829cffd 100644 --- a/web/src/markup.css +++ b/web/src/markup.css @@ -5,11 +5,14 @@ @apply typography-paragraph-regular text-foreground-default; } - .markup > :first-child { + .markup > :first-child, + /* the wrapper hides the render's first child from the rule above, so repeat it inside */ + .markup > [data-markup-key] > :first-child { @apply mt-0; } - .markup > :last-child { + .markup > :last-child, + .markup > [data-markup-key] > :last-child { @apply mb-0; }