diff --git a/apps/roam/package.json b/apps/roam/package.json index 739ab8aec..7aabf89db 100644 --- a/apps/roam/package.json +++ b/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", diff --git a/apps/roam/scripts/__tests__/reactCompatibility.test.ts b/apps/roam/scripts/__tests__/reactCompatibility.test.ts new file mode 100644 index 000000000..d29750a4c --- /dev/null +++ b/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; + renderer: object; + read: () => number; + getHook: () => unknown; +}; + +const buildFixture = async (): Promise => { + 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 => { + 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(); + }, + ); +}); diff --git a/apps/roam/scripts/compile.ts b/apps/roam/scripts/compile.ts index 8c4314360..bb65f1a54 100644 --- a/apps/roam/scripts/compile.ts +++ b/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 = {}, -): 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", diff --git a/apps/roam/scripts/importAsGlobals.ts b/apps/roam/scripts/importAsGlobals.ts new file mode 100644 index 000000000..1613829db --- /dev/null +++ b/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 = {}, +): 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(), + }; + }, + ); + }, + }; +}; diff --git a/apps/roam/scripts/react.cjs b/apps/roam/scripts/react.cjs new file mode 100644 index 000000000..30df4222c --- /dev/null +++ b/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 */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d2a748ce..8393d9435 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -351,8 +351,8 @@ importers: specifier: 3.5.2 version: 3.5.2(react@18.2.0) roamjs-components: - specifier: 0.88.3 - version: 0.88.3(323501797697e57d5f8d07d52746f463) + specifier: 0.90.0 + version: 0.90.0(323501797697e57d5f8d07d52746f463) tldraw: specifier: 2.4.6 version: 2.4.6(patch_hash=56e196052862c9a58a11b43e5e121384cd1d6548416afa0f16e9fbfbf0e4080d)(@types/react-dom@18.2.17)(@types/react@18.2.21)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) @@ -9784,8 +9784,8 @@ packages: deprecated: Rimraf versions prior to v4 are no longer supported hasBin: true - roamjs-components@0.88.3: - resolution: {integrity: sha512-eCKpgKSLoxtOqRZG9c0KOwTQIu2WrYzuipvfeGI377dzUZ8AIj4hJaq6qYzBPL7N1bnns0mMcjHiBhOgLgQTBg==} + roamjs-components@0.90.0: + resolution: {integrity: sha512-lp2a7soqJ8FciuoPsZKOXxciXvsp3IBZIIY/pr/W7nXebDALMeB2N1iz4xS+laPTP057G5TCpVFDS2vEXX438A==} engines: {node: '>=16.0.0', npm: '>=7.0.0'} hasBin: true peerDependencies: @@ -22757,7 +22757,7 @@ snapshots: dependencies: glob: 7.2.3 - roamjs-components@0.88.3(323501797697e57d5f8d07d52746f463): + roamjs-components@0.90.0(323501797697e57d5f8d07d52746f463): dependencies: '@blueprintjs/core': 3.50.4(patch_hash=51c5847e0a73a1be0cc263036ff64d8fada46f3b65831ed938dbca5eecf3edc0)(react-dom@18.2.0(react@18.2.0))(react@18.2.0) '@blueprintjs/datetime': 3.23.14(react-dom@18.2.0(react@18.2.0))(react@18.2.0)