diff --git a/pkg/src/lib/component-manifest-fidelity.integration.test.ts b/pkg/src/lib/component-manifest-fidelity.integration.test.ts index d3cb4bec..e9abf391 100644 --- a/pkg/src/lib/component-manifest-fidelity.integration.test.ts +++ b/pkg/src/lib/component-manifest-fidelity.integration.test.ts @@ -420,6 +420,12 @@ describe.each(ROOTS)("component root $dir", (root) => { config.base = base; serverSidecarRegistry.clear(); clearScopingTemplates(); + // Components are minified as they are loaded (components.ts), so + // the list is this variant's: one kept from another variant would + // pair files loaded under one `minify.html` with pages built under + // the other, which no build does. + invalidateComponentListCache(); + list = await listComponents(root.components); manifest = await buildComponentManifest(list, BascikConfig); }); diff --git a/pkg/src/lib/request-render.test.ts b/pkg/src/lib/request-render.test.ts index 6c5a729e..99357892 100644 --- a/pkg/src/lib/request-render.test.ts +++ b/pkg/src/lib/request-render.test.ts @@ -20,6 +20,7 @@ * `RequestRenderError`, or a once-per-(page, tag) dev log. */ import { beforeEach, describe, expect, it, vi, type Mock } from "vitest"; +import fc from "fast-check"; import { createHash } from "node:crypto"; vi.mock("./config.js", () => ({ @@ -2405,4 +2406,127 @@ describe("usageFinalizer", () => { ); expect(rendered(requestTime)).toBe(rendered(built)); }); + + describe("whitespace around a comment's separator between adjacent instances", () => { + // In a table the build replaces a removed comment with an empty comment + // (the content model there must not change), and of the whitespace runs + // around the comments between two inline elements it keeps one space, at + // the last run. Each instance is minified on its own at request time, so + // the runs on either side of an instance boundary must be reconciled. + const A = `t`; + const instances: Record = { + "leading comment": `\n${A}\n`, + "trailing comment": `${A}\n\n`, + "trailing comment, no final newline": `${A}\n`, + "leading newline": `\n${A}`, + "comment on both sides": `\n${A}\n\n`, + "comment only": `\n`, + "no comment": `${A}\n`, + "block root with a leading comment": `\n
t
\n`, + "two comments": `\n\n${A}\n`, + "comment touching the root": `${A}\n`, + text: `hello\n`, + "text after a comment": `\nhello`, + "text before a comment": `\nhello\n\n`, + "whitespace only": `\n \n`, + empty: ``, + "a script after the root": `${A}\n\n\n`, + }; + const finalize = usageFinalizer({ + minifyHtml: true, + base: "/", + } as ComponentManifest)!; + const names = Object.keys(instances); + const sequences: string[][] = [ + ...names.flatMap((first) => names.map((second) => [first, second])), + ["leading comment", "leading comment", "leading comment"], + ["no comment", "comment only", "leading comment"], + ["trailing comment, no final newline", "comment only", "leading newline"], + ["comment only", "comment only", "no comment"], + ]; + const wrappers: Array<[string, string, string]> = [ + [ + "a table cell", + "

before

", + "

after

", + ], + [ + "a table cell, between inline elements", + "
x", + "y
", + ], + [ + "a formatting element in a table cell", + "
", + "
", + ], + [ + "a
 in a table cell",
+        "
",
+        "
", + ], + ["a paragraph", "

", "

"], + ["a formatting element", "", ""], + ]; + it.each(wrappers)("matches the build in %s", (_label, open, close) => { + for (const sequence of sequences) { + const spans: Array<{ name: string; start: number; end: number }> = []; + let page = open; + for (const _ of sequence) { + spans.push({ + name: "x-c", + start: page.length, + end: page.length + 11, + }); + page += ""; + } + const pieces = sequence.map((name) => instances[name]); + const finished = finalize(page + close, spans, pieces); + expect( + `${open}${finished.join("")}${close}`, + sequence.join(" + "), + ).toBe(minifyHtml(`${open}${pieces.join("")}${close}`)); + } + }); + it("matches the build for any instances written next to each other", () => { + const atom = fc.constantFrom( + "", + "\n", + " ", + A, + "
d
", + "s", + "hello", + "
", + ); + const instance = fc.array(atom, { maxLength: 6 }).map((a) => a.join("")); + fc.assert( + fc.property( + fc.array(instance, { minLength: 2, maxLength: 5 }), + fc.constantFrom(...wrappers), + // Author text on either side of the run, which is left as written. + fc.constantFrom("", "x", "x", "

x

"), + fc.constantFrom("", "y", "y", "

y

"), + (pieces, [, open, close], lead, tail) => { + const spans: Array<{ name: string; start: number; end: number }> = + []; + let page = open + lead; + for (const _ of pieces) { + spans.push({ + name: "x-c", + start: page.length, + end: page.length + 11, + }); + page += ""; + } + const finished = finalize(page + tail + close, spans, pieces); + expect(`${open}${lead}${finished.join("")}${tail}${close}`).toBe( + minifyHtml(`${open}${lead}${pieces.join("")}${tail}${close}`), + ); + }, + ), + { numRuns: 3000 }, + ); + }); + }); }); diff --git a/pkg/src/lib/request-render.ts b/pkg/src/lib/request-render.ts index b0f9b062..c35b6859 100644 --- a/pkg/src/lib/request-render.ts +++ b/pkg/src/lib/request-render.ts @@ -70,11 +70,11 @@ import { DOCUMENT_FRAGMENT_CONTEXT, fragmentContexts, fragmentEdges, - leadingCollapsibleWhitespace, - trailingCollapsibleWhitespace, minifyHtmlFragment, type FragmentContext, + type FragmentNeighbor, } from "./html-minifier.ts"; +import { SEAM_SEPARATOR } from "./html-scanner.ts"; import { applyScopingResult, instantiateTemplate, @@ -499,6 +499,174 @@ export const expandByUsage = ( return pieces.join(""); }; +/** One piece of the whitespace and comment separators at an instance's edge. */ +interface SeamToken { + text: string; + whitespace: boolean; + keep: boolean; +} + +/** + * Split `html` into the whitespace and separators at its start, what lies + * between, and those at its end. A piece that is nothing else is all `lead`. + */ +const splitSeams = ( + html: string, +): { lead: SeamToken[]; middle: string; trail: SeamToken[] } => { + const isSpace = (index: number): boolean => { + const code = html.charCodeAt(index); + return code === 32 || code === 10 || code === 9 || code === 13; + }; + const lead: SeamToken[] = []; + let start = 0; + while (start < html.length) { + if (isSpace(start)) { + let end = start + 1; + while (end < html.length && isSpace(end)) end++; + lead.push({ + text: html.slice(start, end), + whitespace: true, + keep: true, + }); + start = end; + } else if (html.startsWith(SEAM_SEPARATOR, start)) { + lead.push({ text: SEAM_SEPARATOR, whitespace: false, keep: true }); + start += SEAM_SEPARATOR.length; + } else break; + } + const trail: SeamToken[] = []; + let end = html.length; + while (end > start) { + if (isSpace(end - 1)) { + let from = end - 1; + while (from > start && isSpace(from - 1)) from--; + trail.push({ text: html.slice(from, end), whitespace: true, keep: true }); + end = from; + } else if ( + end - SEAM_SEPARATOR.length >= start && + html.startsWith(SEAM_SEPARATOR, end - SEAM_SEPARATOR.length) + ) { + trail.push({ text: SEAM_SEPARATOR, whitespace: false, keep: true }); + end -= SEAM_SEPARATOR.length; + } else break; + } + trail.reverse(); + return { lead, middle: html.slice(start, end), trail }; +}; + +/** What `mergeSeamWhitespace` needs to know about each finished instance. */ +interface SeamNeighbors { + /** What the instance's minified form starts and ends with ("none": nothing). */ + edges: (index: number) => { first: FragmentNeighbor; last: FragmentNeighbor }; + /** What renders before and after each instance, past any that leave nothing. */ + before: readonly FragmentNeighbor[]; + after: readonly FragmentNeighbor[]; +} + +/** + * Reconcile the whitespace where instances written with nothing between them + * meet. Each was minified on its own, so each kept the space its own edge + * called for; in the built page those edges are one stretch of whitespace + * and comment separators (a removed comment leaves an empty one in a table) + * between two things that render, and of that stretch the build keeps: + * + * - whitespace that touches text on either side, which is part of that text; + * - while a formatting element may be open, one space per whitespace run; + * - otherwise one space for every other run together, at the last of them, + * and none when text is on either side of the stretch. + * + * Two runs that now touch are one run. An instance only ever keeps a space + * the build may also keep at that run, so this only removes. Inside a + * `
`, and where the page could not be followed, the instances are as
+ * written and stay so. One pass over the pieces' edges.
+ */
+const mergeSeamWhitespace = (
+  finished: string[],
+  spans: readonly UsageSpan[],
+  contexts: readonly FragmentContext[],
+  neighbors: SeamNeighbors,
+): string[] => {
+  if (finished.length < 2) return finished;
+  let changed = false;
+  /** The stretch being read: its tokens, and what it started after. */
+  let stretch: SeamToken[] = [];
+  let afterText = false;
+  let formatting = false;
+  const close = (beforeText: boolean): void => {
+    const runs: SeamToken[] = [];
+    let previous: SeamToken | undefined;
+    for (const token of stretch) {
+      if (!token.whitespace) {
+        previous = undefined;
+        continue;
+      }
+      if (previous) {
+        token.keep = false;
+        changed = true;
+      } else {
+        runs.push(token);
+        previous = token;
+      }
+    }
+    if (!formatting && runs.length > 0) {
+      // Runs that are part of the text on either side stay with it.
+      const first = stretch[0];
+      const last = stretch[stretch.length - 1];
+      let from = afterText && first.whitespace ? 1 : 0;
+      let to = runs.length;
+      if (beforeText && last.whitespace && runs.length > from) to--;
+      // A single run between two texts is both; it is counted once.
+      if (from > to) from = to;
+      // The build keeps that space only between two inline elements. Each
+      // instance checked its own neighbors, but one that follows text was
+      // minified as if it touched that text, which a comment separator
+      // between them undoes.
+      const keepLast = !beforeText && !afterText;
+      for (let index = from; index < to; index++) {
+        if (keepLast && index === to - 1) break;
+        runs[index].keep = false;
+        changed = true;
+      }
+    }
+    stretch = [];
+  };
+  let open = false;
+  const parts = finished.map((piece, index) => {
+    const context = contexts[index];
+    const skipped = context.inPre || context.verbatim;
+    const adjacent = index > 0 && spans[index].start === spans[index - 1].end;
+    if (open && (skipped || !adjacent)) {
+      // The stretch ran to the end of its run of instances.
+      close(neighbors.after[index - 1] === "text");
+      open = false;
+    }
+    if (skipped) return undefined;
+    if (!open) {
+      open = true;
+      afterText = neighbors.before[index] === "text";
+      formatting = context.formatting;
+    }
+    const seams = splitSeams(piece);
+    stretch.push(...seams.lead);
+    if (seams.middle !== "") {
+      const { first, last } = neighbors.edges(index);
+      close(first === "text");
+      afterText = last === "text";
+      formatting = context.formatting;
+      stretch.push(...seams.trail);
+    }
+    return seams;
+  });
+  if (open) close(neighbors.after[finished.length - 1] === "text");
+  if (!changed) return finished;
+  const text = (tokens: SeamToken[]): string =>
+    tokens.map((token) => (token.keep ? token.text : "")).join("");
+  return finished.map((piece, index) => {
+    const seams = parts[index];
+    return seams ? text(seams.lead) + seams.middle + text(seams.trail) : piece;
+  });
+};
+
 /**
  * The request path's finish for each expanded top-level usage. The build
  * rewrites base paths in, then minifies, the whole page once
@@ -520,42 +688,41 @@ export const usageFinalizer = (
         if (!manifest.minifyHtml) return rebased;
         const contexts = fragmentContexts(html, spans, outer);
         // Two usages written next to each other are each other's neighbors
-        // in the page as their instances, not as their usage tags.
-        const edges = new Map>();
-        const edgesOf = (index: number) => {
-          let found = edges.get(index);
-          if (!found) edges.set(index, (found = fragmentEdges(rebased[index])));
-          return found;
-        };
-        const finished = rebased.map((piece, index) => {
-          const context = { ...contexts[index] };
-          if (context.beforeIsSpan) {
-            const last = edgesOf(index - 1).last;
-            if (last !== "none") context.before = last;
-          }
-          if (context.afterIsSpan) {
-            const first = edgesOf(index + 1).first;
-            if (first !== "none") context.after = first;
-          }
-          return minifyHtmlFragment(piece, context);
-        });
-        // Whitespace at the end of one instance and the start of the next,
-        // written with nothing between them, is one text run in the built
-        // page and collapses to one space; outside a 
 it is kept once.
-        for (let index = 1; index < finished.length; index++) {
-          const context = contexts[index];
-          if (
-            context.inPre ||
-            context.verbatim ||
-            spans[index].start !== spans[index - 1].end ||
-            trailingCollapsibleWhitespace(finished[index - 1]) === 0
-          )
-            continue;
-          finished[index] = finished[index].slice(
-            leadingCollapsibleWhitespace(finished[index]),
-          );
+        // in the page as their instances, not as their usage tags. An
+        // instance that leaves nothing at its edge (it is empty, or only
+        // comments and whitespace) is no neighbor: the one past it is, and
+        // past the last of them whatever the output has there.
+        const edges: Array | undefined> = [];
+        const edgesOf = (index: number) =>
+          (edges[index] ??= fragmentEdges(rebased[index]));
+        // Resolved from the far end of each run of usages, so a run of
+        // instances that leave nothing is walked once, not once per member.
+        const before: FragmentNeighbor[] = [];
+        for (let index = 0; index < rebased.length; index++) {
+          before[index] = contexts[index].before;
+          if (!contexts[index].beforeIsSpan) continue;
+          const last = edgesOf(index - 1).last;
+          before[index] = last !== "none" ? last : before[index - 1];
+        }
+        const after: FragmentNeighbor[] = [];
+        for (let index = rebased.length - 1; index >= 0; index--) {
+          after[index] = contexts[index].after;
+          if (!contexts[index].afterIsSpan) continue;
+          const first = edgesOf(index + 1).first;
+          after[index] = first !== "none" ? first : after[index + 1];
         }
-        return finished;
+        const finished = rebased.map((piece, index) =>
+          minifyHtmlFragment(piece, {
+            ...contexts[index],
+            before: before[index],
+            after: after[index],
+          }),
+        );
+        return mergeSeamWhitespace(finished, spans, contexts, {
+          edges: edgesOf,
+          before,
+          after,
+        });
       }
     : undefined;