Replace timed delay for syntax highlighting with VisibilityObserver

VisibilityObserver gives snappier feedback by skipping the wait. The
downside is that we're highlighting results that are visible only for a
few ms during live search, but it seems like a worthwhile trade for the
perceived speed improvement.

.changeset/gold-seals-divide.md

@@ -0,0 +1,5 @@
+---
+"neogrok": patch
+---
+
+Replace timed delay for syntax highlighting with VisibilityObserver

src/routes/(search-page)/line-group.svelte

@@ -3,82 +3,86 @@
   import { onMount } from "svelte";
   import type { LineGroup } from "./chunk-renderer";
   import RenderedContent from "./rendered-content.svelte";
-  import { writable } from "svelte/store";
   import type { ThemedToken, BundledLanguage } from "shikiji";
 
   export let lines: LineGroup;
   export let file: ResultFile;
 
-  const highlights = writable<
-    undefined | ReadonlyArray<ReadonlyArray<ThemedToken>>
-  >();
-
-  // Syntax highlighting can be pretty CPU intensive and so is done
-  // deferred, during render instead of in the API, and only on the client.
-  // - "Deferred" because most code returned by search is not actually really
-  //   being read by a human, due to live search constantly shuffing results
-  //   out of the UI.
-  // - "During render" because we have computed the much smaller set of lines
-  //   visible by default only at that point, thereby substantially reducing
-  //   the amount of highlighting work to be done. If we did that in the API,
-  //   expanding any collapsed section would require an API request.
+  // Syntax highlighting can be pretty CPU intensive and so is done just in
+  // time, and only on the client.
+  // - "Just in time" because most code returned by search is not actually really
+  //   being read by a human, due to the page being too tall. We use a
+  //   VisibilityObserver to highlight only code that's on the screen.
   // - "Only on the client" because CPU intensive tasks can smoke the server's
   //   performance. Node is great at lots of concurrent I/O, but anything CPU
   //   intensive that blocks its event loop from progressing quickly grinds
   //   things to a halt.
   //
-  // The price to pay is that the client has to download and execute some assets
-  // that are pretty enormous, compared to everything else in the app. The
-  // baseline shikiji bundle (their JS, the oniguruma wasm blob, plus the theme
-  // we use) is something like 300kb _gzipped_, and each language is 5-100 more.
-  onMount(() => {
-    let canceled = false;
+  // The price to pay over doing highlighting on the server is that the client
+  // has to download and execute some assets that are pretty enormous, compared
+  // to everything else in the app. The baseline shikiji bundle (their JS, the
+  // oniguruma wasm blob, plus the theme we use) is something like 300kb
+  // _gzipped_, and each language is 3-50 more.
+  let visible = false;
+  let highlights: undefined | ReadonlyArray<ReadonlyArray<ThemedToken>>;
 
-    const timer = setTimeout(async () => {
-      const { codeToThemedTokens, bundledLanguages } = await import("shikiji");
-      // This is the same normalization that zoekt applies when querying
-      // go-enry, and it seems to be good enough for us querying shikiji as well.
-      let language = file.language.toLowerCase();
-      // ... except when it isn't. go-enry (backed by linguist) and shikiji
-      // (backed by vscode's textmate grammars) just seem to have fundamentally
-      // different lineages, we have no hope but to do some remappings. TODO
-      // perhaps these should be upstreamed as aliases in shikiji.
-      if (language === "restructuredtext") {
-        language = "rst";
-      }
+  const highlight = async (code: string) => {
+    // We dynamically import shiki itself because it's huge and won't be
+    // needed by those landing on the home page with no search query, or
+    // on the server at all.
+    const { codeToThemedTokens, bundledLanguages } = await import("shikiji");
 
-      if (language in bundledLanguages) {
-        // I guess TS isn't interested in doing this refinement // based on the
-        // check above.
-        const lang = language as BundledLanguage;
-        // It's worth checking again, as downloading that chunk can take a
-        // while, and highlighting can occupy meaningful CPU time.
-        if (!canceled) {
-          highlights.set(
-            await codeToThemedTokens(
-              // Shikiji only accepts a single string even though it goes right
-              // ahead and splits it :(.
-              lines.map(({ line: { text } }) => text).join("\n"),
-              {
-                theme: "github-light",
-                lang,
-              },
-            ),
-          );
-        }
-      } else if (language !== "text") {
-        // TODO this will be a lot of console spam... could use a store, as absurd as that is.
-        console.warn(
-          "Could not find shikiji language for '%s', skipping highlighting",
-          language,
-          bundledLanguages,
-        );
-      }
-    }, 500);
+    // This is the same normalization that zoekt applies when querying
+    // go-enry, and it seems to be good enough for us querying shikiji as well.
+    let language = file.language.toLowerCase();
+    // ... except when it isn't. go-enry (backed by linguist) and shikiji
+    // (backed by vscode's textmate grammars) just seem to have fundamentally
+    // different lineages, we have no hope but to do some remappings. TODO
+    // perhaps these should be upstreamed as aliases in shikiji.
+    if (language === "restructuredtext") {
+      language = "rst";
+    }
 
+    if (language in bundledLanguages) {
+      // I guess TS isn't interested in doing this refinement based on the
+      // check above.
+      const lang = language as BundledLanguage;
+      // It's worth checking again, as downloading that chunk can take a
+      // while, and highlighting can occupy meaningful CPU time.
+      highlights = await codeToThemedTokens(code, {
+        theme: "github-light",
+        lang,
+      });
+    } else if (language !== "text") {
+      // TODO this will be a lot of console spam... could use a store, as
+      // absurd as that is.
+      console.warn(
+        "Could not find shikiji language for '%s', skipping highlighting",
+        language,
+        bundledLanguages,
+      );
+    }
+  };
+
+  $: {
+    if (visible) {
+      // Shikiji only accepts a single string even though it goes
+      // right ahead and splits it :(.
+      highlight(lines.map(({ line: { text } }) => text).join("\n"));
+    }
+  }
+
+  let visibilityCanary: Element;
+  onMount(() => {
+    const observer = new IntersectionObserver(async (entries) => {
+      if (entries.some(({ isIntersecting }) => isIntersecting)) {
+        observer.disconnect();
+        visible = true;
+      }
+    });
+    observer.observe(visibilityCanary);
     return () => {
-      canceled = true;
-      clearTimeout(timer);
+      observer.disconnect();
     };
   });
 </script>
@@ -90,6 +94,7 @@
     the overwhelming majority of cases.
 -->
 <div
+  bind:this={visibilityCanary}
   class="py-1 grid grid-cols-[minmax(2rem,_min-content)_1fr] gap-x-2 whitespace-pre overflow-x-auto"
 >
   {#each lines as { lineNumber, line }, i}
@@ -103,7 +108,6 @@
         >
       {:else}{lineNumber}{/if}
     </span>
-    <code><RenderedContent content={line} highlights={$highlights?.[i]} /></code
-    >
+    <code><RenderedContent content={line} highlights={highlights?.[i]} /></code>
   {/each}
 </div>

src/routes/(search-page)/search-results-file.svelte

@@ -64,7 +64,7 @@
     <SearchResultsFileHeader {file} {rank} />
     {#if lineGroups.length > 0}
       <div class="font-mono text-sm divide-y">
-        {#each lineGroups as lines (lines[0].lineNumber)}
+        {#each lineGroups as lines}
           <LineGroup {lines} {file} />
         {/each}
       </div>