ENG-2211 Fix Roam direct canvas load race

apps/roam/package.json

@@ -84,7 +84,7 @@
     "react-draggable": "4.4.5",
     "react-in-viewport": "1.0.0-alpha.20",
     "react-vertical-timeline-component": "3.5.2",
-    "roamjs-components": "0.88.3",
+    "roamjs-components": "0.90.0",
     "tldraw": "2.4.6",
     "use-sync-external-store": "1.5.0",
     "xregexp": "^5.0.0",

apps/roam/scripts/__tests__/reactCompatibility.test.ts

@@ -0,0 +1,99 @@
+import esbuild from "esbuild";
+import { runInNewContext } from "node:vm";
+import { describe, expect, it, vi } from "vitest";
+import { importAsGlobals } from "../importAsGlobals";
+
+type Fixture = {
+  react: Record<string, unknown>;
+  renderer: object;
+  read: () => number;
+  getHook: () => unknown;
+};
+
+const buildFixture = async (): Promise<string> => {
+  const result = await esbuild.build({
+    bundle: true,
+    stdin: {
+      contents: `
+            import React, { useSyncExternalStore } from "react";
+            import ReactDOM from "react-dom";
+            export const react = React;
+            export const renderer = ReactDOM;
+            export const getHook = () => useSyncExternalStore;
+            export const read = () => useSyncExternalStore(() => () => {}, () => 42);
+          `,
+      resolveDir: process.cwd(),
+    },
+    format: "cjs",
+    write: false,
+    sourcemap: false,
+    define: { "process.env.NODE_ENV": '"production"' },
+    plugins: [
+      importAsGlobals({
+        react: "./scripts/react.cjs",
+        "react-dom": "window.ReactDOM",
+      }),
+    ],
+  });
+  return result.outputFiles?.[0]?.text || "";
+};
+
+describe("Roam React host bundle", () => {
+  it.each([false, true])(
+    "keeps the initial host hook stable when it is already replaced: %s",
+    async (hasExistingShim): Promise<void> => {
+      const initialHook = vi.fn((_subscribe, getSnapshot: () => number) =>
+        getSnapshot(),
+      );
+      const useState = vi.fn((value: unknown) => [value, vi.fn()]);
+      const dispatcher = {};
+      const hostReact = {
+        useState,
+        useEffect: vi.fn(),
+        useLayoutEffect: vi.fn(),
+        useDebugValue: vi.fn(),
+        createElement: vi.fn(),
+        __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: dispatcher,
+        version: "18.2.0",
+        useSyncExternalStore: hasExistingShim
+          ? vi.fn(initialHook)
+          : initialHook,
+      };
+      const capturedHook = hostReact.useSyncExternalStore;
+      const renderer = {};
+      const module = { exports: {} as Fixture };
+      runInNewContext(await buildFixture(), {
+        module,
+        exports: module.exports,
+        window: {
+          React: hostReact,
+          ReactDOM: renderer,
+          document: { createElement: vi.fn() },
+        },
+      });
+      const fixture = module.exports;
+      const privateHook = fixture.getHook();
+
+      expect(fixture.react).not.toBe(hostReact);
+      expect(fixture.react.useState).toBe(hostReact.useState);
+      expect(fixture.react.createElement).toBe(hostReact.createElement);
+      expect(
+        fixture.react.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED,
+      ).toBe(dispatcher);
+      expect(fixture.renderer).toBe(renderer);
+      expect(hostReact.useSyncExternalStore).toBe(capturedHook);
+      expect(privateHook).toBe(capturedHook);
+      expect(fixture.read()).toBe(42);
+      expect(initialHook).toHaveBeenCalledTimes(1);
+
+      // Simulate another extension loading between component renders.
+      const replacement = vi.fn(() => -2);
+      hostReact.useSyncExternalStore = replacement;
+      expect(fixture.getHook()).toBe(privateHook);
+      expect(fixture.read()).toBe(42);
+      expect(initialHook).toHaveBeenCalledTimes(2);
+      expect(useState).not.toHaveBeenCalled();
+      expect(replacement).not.toHaveBeenCalled();
+    },
+  );
+});

apps/roam/scripts/compile.ts

@@ -3,6 +3,7 @@ import { execSync } from "child_process";
 import fs from "fs";
 import path from "path";
 import { z } from "zod";
+import { importAsGlobals } from "./importAsGlobals";
 
 const getVersion = (): string => {
   try {
@@ -64,57 +65,6 @@ try {
   throw error;
 }
 
-// https://github.com/evanw/esbuild/issues/337#issuecomment-954633403
-const importAsGlobals = (
-  mapping: Record<string, string> = {},
-): esbuild.Plugin => {
-  const escRe = (s: string) => s.replace(/[-\/\\^$*+?.()|[\]{}]/g, "\\$&");
-  const filter = new RegExp(
-    Object.keys(mapping).length
-      ? Object.keys(mapping)
-          .map((mod) => `^${escRe(mod)}$`)
-          .join("|")
-      : /$^/,
-  );
-
-  return {
-    name: "global-imports",
-    setup(build) {
-      build.onResolve({ filter }, (args) => {
-        if (!mapping[args.path]) {
-          throw new Error("Unknown global: " + args.path);
-        }
-        return {
-          path: args.path,
-          namespace: "external-global",
-        };
-      });
-
-      build.onLoad(
-        {
-          filter,
-          namespace: "external-global",
-        },
-        async (args) => {
-          const global = mapping[args.path];
-          if (fs.existsSync(global)) {
-            return {
-              contents: fs.readFileSync(global).toString(),
-              loader: "js",
-              resolveDir: path.dirname(global),
-            };
-          }
-          return {
-            contents: `module.exports = ${global};`,
-            loader: "js",
-            resolveDir: process.cwd(),
-          };
-        },
-      );
-    },
-  };
-};
-
 const DEFAULT_FILES_INCLUDED = ["package.json", "README.md"];
 
 const addPlaceholderChangelogPlugin = (outdir: string): esbuild.Plugin => ({
@@ -156,7 +106,7 @@ export const args = {
     "marked=window.RoamLazy.Marked",
     "marked-react=window.RoamLazy.MarkedReact",
     "nanoid=window.Nanoid;module.exports.nanoid=window.Nanoid",
-    'react=window.React;module.exports.useSyncExternalStore=require("use-sync-external-store/shim").useSyncExternalStore',
+    "react=./scripts/react.cjs",
     "react/jsx-runtime=./node_modules/react/jsx-runtime.js",
     "react-dom=window.ReactDOM",
     "react-youtube=window.ReactYoutube",

apps/roam/scripts/importAsGlobals.ts

@@ -0,0 +1,54 @@
+import esbuild from "esbuild";
+import fs from "fs";
+import path from "path";
+
+// https://github.com/evanw/esbuild/issues/337#issuecomment-954633403
+export const importAsGlobals = (
+  mapping: Record<string, string> = {},
+): esbuild.Plugin => {
+  const escRe = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+  const filter = new RegExp(
+    Object.keys(mapping).length
+      ? Object.keys(mapping)
+          .map((mod) => `^${escRe(mod)}$`)
+          .join("|")
+      : /$^/,
+  );
+
+  return {
+    name: "global-imports",
+    setup: (build): void => {
+      build.onResolve({ filter }, (args) => {
+        if (!mapping[args.path]) {
+          throw new Error("Unknown global: " + args.path);
+        }
+        return {
+          path: args.path,
+          namespace: "external-global",
+        };
+      });
+
+      build.onLoad(
+        {
+          filter,
+          namespace: "external-global",
+        },
+        (args) => {
+          const global = mapping[args.path];
+          if (fs.existsSync(global)) {
+            return {
+              contents: fs.readFileSync(global).toString(),
+              loader: "js",
+              resolveDir: path.dirname(global),
+            };
+          }
+          return {
+            contents: `module.exports = ${global};`,
+            loader: "js",
+            resolveDir: process.cwd(),
+          };
+        },
+      );
+    },
+  };
+};

apps/roam/scripts/react.cjs

@@ -0,0 +1,4 @@
+// Capture Roam's React exports once so later extension assignments cannot change
+// the hook implementation used by an already mounted DG component.
+module.exports = { ...window.React };
+/* global module */