diff --git a/apps/app/src/components/plugin/PluginPanelHeader.tsx b/apps/app/src/components/plugin/PluginPanelHeader.tsx index ded47814fa..4f55483f23 100644 --- a/apps/app/src/components/plugin/PluginPanelHeader.tsx +++ b/apps/app/src/components/plugin/PluginPanelHeader.tsx @@ -1,6 +1,7 @@ import { Component, type ReactNode } from "react"; import type { PluginNavPanelChrome } from "@/lib/plugin-nav-panel-chrome"; import type { PluginNavPanelSlot } from "@/lib/plugin-slots"; +import { usePluginCss } from "@/lib/plugin-css"; import { PluginIcon } from "./PluginIcon"; import { PluginContext } from "./plugin-context"; import { useOptionalPaneContext } from "@/views/thread-detail/PaneContext"; @@ -78,6 +79,7 @@ export function PluginPanelHeaderActions({ }) { const paneContext = useOptionalPaneContext(); const HeaderContent = panel.headerContent; + usePluginCss(HeaderContent === undefined ? null : panel.pluginId); const panelStateId = getPluginPagePanelStateId({ panelPath: panel.path, paneId: paneId ?? paneContext?.paneId, diff --git a/apps/app/src/components/plugin/PluginSlotMount.test.tsx b/apps/app/src/components/plugin/PluginSlotMount.test.tsx index ca70ad3bcd..6cac4f15f8 100644 --- a/apps/app/src/components/plugin/PluginSlotMount.test.tsx +++ b/apps/app/src/components/plugin/PluginSlotMount.test.tsx @@ -1,12 +1,14 @@ // @vitest-environment jsdom -import { cleanup, render, screen } from "@testing-library/react"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import { createPortal } from "react-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { PluginSlotMount, resetAllCrashedPluginSlotsForTest, resetCrashedPluginSlots, } from "./PluginSlotMount"; +import { applyPluginCss, resetPluginCssForTest } from "@/lib/plugin-css"; function Bomb(): never { throw new Error("kaboom"); @@ -19,6 +21,7 @@ function Healthy() { describe("PluginSlotMount", () => { beforeEach(() => { resetAllCrashedPluginSlotsForTest(); + resetPluginCssForTest(); // React logs boundary-caught errors; keep test output quiet. vi.spyOn(console, "error").mockImplementation(() => {}); vi.spyOn(console, "warn").mockImplementation(() => {}); @@ -26,6 +29,7 @@ describe("PluginSlotMount", () => { afterEach(() => { cleanup(); + resetPluginCssForTest(); vi.restoreAllMocks(); }); @@ -49,6 +53,46 @@ describe("PluginSlotMount", () => { expect(screen.getByText("healthy slot")).toBeDefined(); }); + it("keeps one sheet through simultaneous mounts and a portal until the final route unmount", async () => { + applyPluginCss("demo", "/demo.css?h=v1"); + function PortalContent() { + return createPortal(
portalled plugin content
, document.body); + } + const view = render( + <> + + + + + + + , + ); + const pluginSheets = () => + document.head.querySelectorAll('link[data-bb-plugin-css="demo"]'); + expect(pluginSheets()).toHaveLength(1); + expect(screen.getByText("portalled plugin content")).toBeDefined(); + + view.rerender( + + + , + ); + expect(pluginSheets()).toHaveLength(1); + + view.unmount(); + await act(async () => {}); + expect(pluginSheets()).toHaveLength(0); + }); + it("keeps a crashed slot instance disabled for the session across remounts", () => { const first = render( diff --git a/apps/app/src/components/plugin/PluginSlotMount.tsx b/apps/app/src/components/plugin/PluginSlotMount.tsx index 084bfd555f..1b5123bca7 100644 --- a/apps/app/src/components/plugin/PluginSlotMount.tsx +++ b/apps/app/src/components/plugin/PluginSlotMount.tsx @@ -1,5 +1,6 @@ import { Component, type ErrorInfo, type ReactNode } from "react"; import { Pill } from "@bb/shared-ui/pill"; +import { usePluginCss } from "@/lib/plugin-css"; import { PluginContext, PluginSlotOwnershipContext, @@ -223,6 +224,7 @@ export function PluginSlotMount({ instanceId, onCrash, }: PluginSlotMountProps) { + usePluginCss(pluginId); return ( { resetPluginFrontendBootStateForTest(); window.localStorage.clear(); resetAllCrashedPluginSlotsForTest(); + resetPluginCssForTest(); vi.restoreAllMocks(); }); @@ -1311,6 +1313,58 @@ describe("PluginNavSidebarItems + PluginPanelView", () => { expect(screen.getByText("board panel body")).toBeDefined(); }); + it("releases the plugin stylesheet when navigation unmounts the panel route", async () => { + setPluginSlotRegistrations( + "demo", + registrationSet({ + navPanels: [ + { + id: "board", + title: "Demo board", + icon: "columns", + path: "board", + component: Board, + }, + ], + }), + ); + applyPluginCss("demo", "/demo.css?h=route"); + function LeavePanel() { + const navigate = useNavigate(); + return ( + + ); + } + render( + + + + + + + } + /> + home} /> + + , + ); + expect( + document.head.querySelector('link[data-bb-plugin-css="demo"]'), + ).not.toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Leave panel" })); + await act(async () => {}); + expect(screen.getByText("home")).toBeDefined(); + expect( + document.head.querySelector('link[data-bb-plugin-css="demo"]'), + ).toBeNull(); + }); + it("shows a plugin panel's position when it is open in a split", () => { setPluginSlotRegistrations( "demo", @@ -1534,12 +1588,13 @@ describe("plugin panel shared title bar and full-bleed body", () => { expect(screen.queryByText(/plugin demo crashed/)).toBeNull(); }); - it("always renders the shared title and headerContent", () => { + it("gives headerContent independent CSS ownership without a mounted panel body", async () => { function Accessory() { return ; } const panel = panelSlot({ headerContent: Accessory }); - render( + applyPluginCss("demo", "/demo.css?h=header"); + const view = render( <> @@ -1549,6 +1604,16 @@ describe("plugin panel shared title bar and full-bleed body", () => { expect( screen.getByRole("button", { name: "Toggle sidebar" }), ).toBeDefined(); + expect( + document.head.querySelector('link[data-bb-plugin-css="demo"]'), + ).not.toBeNull(); + expect(screen.queryByTestId("plugin-panel-body")).toBeNull(); + + view.unmount(); + await act(async () => {}); + expect( + document.head.querySelector('link[data-bb-plugin-css="demo"]'), + ).toBeNull(); }); it("keys the right-panel toggle target to its owning pane", () => { diff --git a/apps/app/src/lib/plugin-css.ts b/apps/app/src/lib/plugin-css.ts new file mode 100644 index 0000000000..b86e3f6cff --- /dev/null +++ b/apps/app/src/lib/plugin-css.ts @@ -0,0 +1,222 @@ +import { useInsertionEffect } from "react"; + +const CSS_MARKER = "data-bb-plugin-css"; +const CSS_PRELOAD_MARKER = "data-bb-plugin-css-preload"; + +interface PluginCssRecord { + consumers: number; + cleanupEpoch: number; + loadedUrl: string | null; + pendingStylesheet: HTMLLinkElement | null; + preload: HTMLLinkElement | null; + stylesheet: HTMLLinkElement | null; + url: string | null; +} + +const recordsByPluginId = new Map(); + +function recordFor(pluginId: string): PluginCssRecord { + const existing = recordsByPluginId.get(pluginId); + if (existing !== undefined) return existing; + const created: PluginCssRecord = { + consumers: 0, + cleanupEpoch: 0, + loadedUrl: null, + pendingStylesheet: null, + preload: null, + stylesheet: null, + url: null, + }; + recordsByPluginId.set(pluginId, created); + return created; +} + +function linkUrl(link: HTMLLinkElement | null): string | null { + return link?.getAttribute("href") ?? null; +} + +function removeLink(link: HTMLLinkElement | null): void { + link?.remove(); +} + +function warnLoadFailure(pluginId: string, url: string): void { + console.warn(`bb plugin "${pluginId}": failed to load stylesheet ${url}`); +} + +function startPreload( + pluginId: string, + record: PluginCssRecord, + url: string, +): void { + if (record.loadedUrl === url || linkUrl(record.preload) === url) return; + removeLink(record.preload); + const link = document.createElement("link"); + link.rel = "preload"; + link.as = "style"; + link.fetchPriority = "low"; + link.href = url; + link.setAttribute(CSS_PRELOAD_MARKER, pluginId); + record.preload = link; + link.onload = () => { + link.remove(); + if (record.preload === link) record.preload = null; + if (record.url !== url) return; + record.loadedUrl = url; + if (record.consumers > 0) activateStylesheet(pluginId, record, url); + }; + link.onerror = () => { + link.remove(); + if (record.preload === link) record.preload = null; + if (record.url === url) warnLoadFailure(pluginId, url); + }; + document.head.appendChild(link); +} + +function activateStylesheet( + pluginId: string, + record: PluginCssRecord, + url: string, +): void { + if (linkUrl(record.pendingStylesheet) === url) return; + if (linkUrl(record.stylesheet) === url) { + record.loadedUrl = url; + removeLink(record.preload); + record.preload = null; + return; + } + + removeLink(record.preload); + record.preload = null; + removeLink(record.pendingStylesheet); + + const previous = record.stylesheet; + const link = document.createElement("link"); + link.rel = "stylesheet"; + link.href = url; + link.setAttribute(CSS_MARKER, pluginId); + record.pendingStylesheet = link; + link.onload = () => { + if ( + record.pendingStylesheet !== link || + record.url !== url || + record.consumers === 0 + ) { + link.remove(); + return; + } + previous?.remove(); + record.stylesheet = link; + record.pendingStylesheet = null; + record.loadedUrl = url; + }; + link.onerror = () => { + link.remove(); + if (record.pendingStylesheet === link) record.pendingStylesheet = null; + if (record.url === url) warnLoadFailure(pluginId, url); + }; + document.head.appendChild(link); +} + +function deactivateStylesheet(record: PluginCssRecord): void { + removeLink(record.pendingStylesheet); + removeLink(record.stylesheet); + record.pendingStylesheet = null; + record.stylesheet = null; +} + +function deactivateAfterFinalRelease( + pluginId: string, + record: PluginCssRecord, +): void { + const cleanupEpoch = ++record.cleanupEpoch; + queueMicrotask(() => { + if (record.cleanupEpoch !== cleanupEpoch || record.consumers > 0) return; + deactivateStylesheet(record); + if (record.url === null) { + recordsByPluginId.delete(pluginId); + return; + } + startPreload(pluginId, record, record.url); + }); +} + +/** + * Publish the stylesheet URL for the current frontend generation. + * + * An inactive bundle warms its immutable response with a low-priority preload + * and removes that link after it settles. Mounted plugin UI owns a real + * stylesheet through {@link retainPluginCss}; the final release removes it. + * A changed URL loads beside the active sheet and replaces it only after the + * new response succeeds, so a failed live reload leaves the old CSS usable. + */ +export function applyPluginCss(pluginId: string, url: string | null): void { + const record = recordFor(pluginId); + if (url === null) { + record.cleanupEpoch += 1; + record.url = null; + record.loadedUrl = null; + removeLink(record.preload); + record.preload = null; + deactivateStylesheet(record); + if (record.consumers === 0) recordsByPluginId.delete(pluginId); + return; + } + + if (record.url === url) { + if (record.consumers > 0) activateStylesheet(pluginId, record, url); + else startPreload(pluginId, record, url); + return; + } + + record.cleanupEpoch += 1; + record.url = url; + record.loadedUrl = linkUrl(record.stylesheet) === url ? url : null; + removeLink(record.preload); + record.preload = null; + if (record.consumers > 0) { + activateStylesheet(pluginId, record, url); + return; + } + deactivateStylesheet(record); + startPreload(pluginId, record, url); +} + +/** Keep one plugin stylesheet active until the returned release is called. */ +export function retainPluginCss(pluginId: string): () => void { + const record = recordFor(pluginId); + record.cleanupEpoch += 1; + record.consumers += 1; + if (record.url !== null) activateStylesheet(pluginId, record, record.url); + let released = false; + return () => { + if (released) return; + released = true; + record.consumers = Math.max(0, record.consumers - 1); + if (record.consumers === 0) { + deactivateAfterFinalRelease(pluginId, record); + } + }; +} + +/** Activate cached plugin CSS before React lays out or paints a scoped slot. */ +export function usePluginCss(pluginId: string | null): void { + useInsertionEffect( + () => (pluginId === null ? undefined : retainPluginCss(pluginId)), + [pluginId], + ); +} + +/** Test-only. */ +export function resetPluginCssForTest(): void { + for (const record of recordsByPluginId.values()) { + record.cleanupEpoch += 1; + removeLink(record.preload); + deactivateStylesheet(record); + } + recordsByPluginId.clear(); + for (const link of document.head.querySelectorAll( + `link[${CSS_MARKER}], link[${CSS_PRELOAD_MARKER}]`, + )) { + link.remove(); + } +} diff --git a/apps/app/src/lib/plugin-frontend-load-order.test.ts b/apps/app/src/lib/plugin-frontend-load-order.test.ts index f12f1c79f5..f67a158717 100644 --- a/apps/app/src/lib/plugin-frontend-load-order.test.ts +++ b/apps/app/src/lib/plugin-frontend-load-order.test.ts @@ -3,7 +3,6 @@ import { describe, expect, it, vi } from "vitest"; import { definePluginApp } from "./plugin-app-definition"; import { - createBatchedPluginCssApplier, createPluginFrontendReconcileState, orderPluginFrontendCandidates, PLUGIN_FRONTEND_LOAD_CONCURRENCY, @@ -63,6 +62,7 @@ function makeDeps( fetchCandidates: async () => candidates, importModule: async () => pluginModule(), applyCss: vi.fn(), + retainCss: vi.fn(() => vi.fn()), resetCrashedSlots: vi.fn(), setRegistrations: vi.fn(), removeRegistrations: vi.fn(), @@ -165,59 +165,3 @@ describe("reconcilePluginFrontends load scheduling", () => { expect(state.records.get("also")?.status).toBe("loaded"); }); }); - -describe("createBatchedPluginCssApplier", () => { - it("coalesces insertions that land before the next frame into one flush", () => { - const apply = vi.fn(); - const frames: Array<() => void> = []; - const applyCss = createBatchedPluginCssApplier({ - apply, - requestFrame: (callback) => { - frames.push(callback); - }, - }); - - applyCss("a", "/a.css"); - applyCss("b", "/b.css"); - applyCss("a", "/a2.css"); // newer URL for the same plugin replaces - expect(apply).not.toHaveBeenCalled(); - expect(frames).toHaveLength(1); - - frames[0]!(); - expect(apply.mock.calls).toEqual([ - ["a", "/a2.css"], - ["b", "/b.css"], - ]); - - // A later insertion requests a fresh frame rather than being dropped. - applyCss("c", "/c.css"); - expect(frames).toHaveLength(2); - frames[1]!(); - expect(apply).toHaveBeenLastCalledWith("c", "/c.css"); - }); - - it("removes synchronously and cancels a pending insertion for that plugin", () => { - const apply = vi.fn(); - const frames: Array<() => void> = []; - const applyCss = createBatchedPluginCssApplier({ - apply, - requestFrame: (callback) => { - frames.push(callback); - }, - }); - - applyCss("a", "/a.css"); - applyCss("b", "/b.css"); - applyCss("a", null); - // Teardown/disposal must not leave a sheet behind, so removal does not - // wait for the frame. - expect(apply.mock.calls).toEqual([["a", null]]); - - frames[0]!(); - // The pending insertion for "a" was cancelled by the removal. - expect(apply.mock.calls).toEqual([ - ["a", null], - ["b", "/b.css"], - ]); - }); -}); diff --git a/apps/app/src/lib/plugin-frontend-reload.test.ts b/apps/app/src/lib/plugin-frontend-reload.test.ts index bbdaefb61e..50ad80459f 100644 --- a/apps/app/src/lib/plugin-frontend-reload.test.ts +++ b/apps/app/src/lib/plugin-frontend-reload.test.ts @@ -3,8 +3,9 @@ import type { PluginComposerThreadRowStatus } from "@get-bb/plugin-sdk"; import { createElement } from "react"; import { createRoot } from "react-dom/client"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; import { act } from "@testing-library/react"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, type Mock, vi } from "vitest"; import { installForeignDomMutationGuard, uninstallForeignDomMutationGuardForTest, @@ -19,16 +20,22 @@ import { type PluginFrontendCandidate, type PluginFrontendReconcileDeps, } from "./plugin-frontend"; +import { resetPluginCssForTest, retainPluginCss } from "./plugin-css"; import { getPluginSlotSnapshot, removePluginSlotRegistrations, resetPluginSlotStoreForTest, setPluginSlotRegistrations, + usePluginSlots, } from "./plugin-slots"; import { getPluginThreadRowStatus, resetPluginThreadRowStatusesForTest, } from "./plugin-thread-row-status"; +import { PluginSlotMount } from "@/components/plugin/PluginSlotMount"; +import { PLUGIN_PANEL_ROUTE_PATH } from "./route-paths"; +import { applyAppThemeCss } from "./themes"; +import { PluginPanelView } from "@/views/PluginPanelView"; function candidate( pluginId: string, @@ -71,10 +78,36 @@ function contentScriptModule( afterEach(() => { resetPluginThreadRowStatusesForTest(); + resetPluginSlotStoreForTest(); + resetPluginCssForTest(); uninstallForeignDomMutationGuardForTest(); }); -function makeDeps(initial: PluginFrontendCandidate[] = []) { +function MountedHomepageSections() { + const { homepageSections } = usePluginSlots(); + return createElement( + "div", + null, + ...homepageSections.map((section) => + createElement(PluginSlotMount, { + key: `${section.pluginId}/${section.id}/${section.generation}`, + pluginId: section.pluginId, + slotKind: "homepageSection", + slotId: section.id, + children: createElement(section.component, { projectId: null }), + }), + ), + ); +} + +interface TestReconcileDeps extends PluginFrontendReconcileDeps { + fetchCandidates: Mock<() => Promise>; + importModule: Mock<(url: string) => Promise>; + removeRegistrations: Mock; + setRegistrations: Mock; +} + +function makeDeps(initial: PluginFrontendCandidate[] = []): TestReconcileDeps { return { fetchCandidates: vi.fn( async (): Promise => initial, @@ -83,6 +116,7 @@ function makeDeps(initial: PluginFrontendCandidate[] = []) { async (_url: string): Promise => pluginModule("hello"), ), applyCss: vi.fn(), + retainCss: vi.fn(() => vi.fn()), resetCrashedSlots: vi.fn(), setRegistrations: vi.fn(), removeRegistrations: vi.fn(), @@ -90,7 +124,7 @@ function makeDeps(initial: PluginFrontendCandidate[] = []) { warn: vi.fn(), routePluginId: () => null, mountTimeoutMs: undefined as number | undefined, - } satisfies PluginFrontendReconcileDeps; + }; } describe("reconcilePluginFrontends", () => { @@ -152,6 +186,7 @@ describe("reconcilePluginFrontends", () => { fetchCandidates, importModule: async () => pluginModule("hello"), applyCss: vi.fn(), + retainCss: vi.fn(() => vi.fn()), resetCrashedSlots: vi.fn(), setRegistrations: setPluginSlotRegistrations, removeRegistrations: removePluginSlotRegistrations, @@ -177,6 +212,171 @@ describe("reconcilePluginFrontends", () => { resetPluginSlotStoreForTest(); }); + it("publishes CSS before a cold deep-link panel registration can render", async () => { + const state = createPluginFrontendReconcileState(); + const preparedDuringRender = vi.fn(); + const deps = makeDeps([candidate("hello", "cold")]); + deps.applyCss = applyPluginCss; + deps.retainCss = retainPluginCss; + deps.setRegistrations = vi.fn(setPluginSlotRegistrations); + deps.removeRegistrations = vi.fn(removePluginSlotRegistrations); + deps.importModule.mockResolvedValue({ + default: definePluginApp((app) => { + app.slots.navPanel({ + id: "panel", + icon: "PanelTop", + path: "panel", + title: "Cold panel", + component: ({ subPath }) => { + const prepared = document.head.querySelector( + 'link[data-bb-plugin-css-preload="hello"], link[data-bb-plugin-css="hello"]', + ); + preparedDuringRender(prepared?.getAttribute("href") ?? null); + return createElement("div", null, `cold panel body:${subPath}`); + }, + }); + }), + }); + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + createElement( + MemoryRouter, + { initialEntries: ["/plugins/hello/panel/notes/today.md"] }, + createElement( + Routes, + null, + createElement(Route, { + path: PLUGIN_PANEL_ROUTE_PATH, + element: createElement(PluginPanelView), + }), + ), + ), + ); + }); + + await act(async () => { + await reconcilePluginFrontends(state, deps); + }); + + expect(preparedDuringRender).toHaveBeenCalledWith( + "/api/v1/plugins/hello/assets/app.css?h=cold", + ); + expect(container.textContent).toContain("cold panel body:notes/today.md"); + expect( + document.head.querySelector('link[data-bb-plugin-css="hello"]'), + ).not.toBeNull(); + + act(() => root.unmount()); + container.remove(); + }); + + it("retains CSS for a content script's whole generation, including cleanup", async () => { + const state = createPluginFrontendReconcileState(); + const deps = makeDeps([candidate("shell-owner", "v1")]); + const events: string[] = []; + const stylesheetIsActive = () => + document.head.querySelector('link[data-bb-plugin-css="shell-owner"]') !== + null; + deps.applyCss = applyPluginCss; + deps.retainCss = retainPluginCss; + deps.importModule.mockResolvedValue( + contentScriptModule((app) => { + app.contentScripts.register({ + id: "shell-dom", + mount() { + events.push(`mount:${stylesheetIsActive()}`); + return () => { + events.push(`dispose:${stylesheetIsActive()}`); + }; + }, + }); + }), + ); + + await reconcilePluginFrontends(state, deps); + expect(events).toEqual(["mount:true"]); + expect(stylesheetIsActive()).toBe(true); + + deps.fetchCandidates.mockResolvedValue([]); + await reconcilePluginFrontends(state, deps); + expect(events).toEqual(["mount:true", "dispose:true"]); + expect(stylesheetIsActive()).toBe(false); + }); + + it("keeps the active sheet through a real generation reload and a failed CSS replacement", async () => { + const state = createPluginFrontendReconcileState(); + const deps = makeDeps([candidate("hello", "v1")]); + deps.applyCss = applyPluginCss; + deps.retainCss = retainPluginCss; + deps.setRegistrations = vi.fn(setPluginSlotRegistrations); + deps.removeRegistrations = vi.fn(removePluginSlotRegistrations); + deps.importModule.mockImplementation(async (url: string) => { + const version = /[?&]h=([^&]+)/.exec(url)?.[1] ?? "unknown"; + return { + default: definePluginApp((app) => { + app.slots.homepageSection({ + id: "section", + title: version, + component: () => + createElement("div", null, `generation ${version}`), + }); + }), + }; + }); + const links = () => [ + ...document.head.querySelectorAll( + 'link[data-bb-plugin-css="hello"]', + ), + ]; + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + act(() => root.render(createElement(MountedHomepageSections))); + + await act(async () => { + await reconcilePluginFrontends(state, deps); + }); + expect(container.textContent).toContain("generation v1"); + links()[0]?.dispatchEvent(new Event("load")); + + deps.fetchCandidates.mockResolvedValue([candidate("hello", "v2")]); + await act(async () => { + await reconcilePluginFrontends(state, deps); + }); + expect(container.textContent).toContain("generation v2"); + expect(links().map((link) => link.getAttribute("href"))).toEqual([ + "/api/v1/plugins/hello/assets/app.css?h=v1", + "/api/v1/plugins/hello/assets/app.css?h=v2", + ]); + + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + links()[1]?.dispatchEvent(new Event("error")); + expect(links().map((link) => link.getAttribute("href"))).toEqual([ + "/api/v1/plugins/hello/assets/app.css?h=v1", + ]); + expect(container.textContent).toContain("generation v2"); + warn.mockRestore(); + + deps.fetchCandidates.mockResolvedValue([candidate("hello", "v3")]); + await act(async () => { + await reconcilePluginFrontends(state, deps); + }); + expect(links().map((link) => link.getAttribute("href"))).toEqual([ + "/api/v1/plugins/hello/assets/app.css?h=v1", + "/api/v1/plugins/hello/assets/app.css?h=v3", + ]); + links()[1]?.dispatchEvent(new Event("load")); + expect(links().map((link) => link.getAttribute("href"))).toEqual([ + "/api/v1/plugins/hello/assets/app.css?h=v3", + ]); + + act(() => root.unmount()); + container.remove(); + }); + it("drops registrations, CSS, and record when a plugin disappears from the inventory", async () => { const state = createPluginFrontendReconcileState(); const deps = makeDeps([candidate("hello", "v1")]); @@ -809,11 +1009,7 @@ describe("reconcilePluginFrontends", () => { describe("applyPluginCss", () => { afterEach(() => { - for (const link of [ - ...document.head.querySelectorAll("link[data-bb-plugin-css]"), - ]) { - link.remove(); - } + resetPluginCssForTest(); }); function links(pluginId: string): HTMLLinkElement[] { @@ -824,9 +1020,19 @@ describe("applyPluginCss", () => { ]; } + function preloads(pluginId: string): HTMLLinkElement[] { + return [ + ...document.head.querySelectorAll( + `link[data-bb-plugin-css-preload="${pluginId}"]`, + ), + ]; + } + it("keeps the old link until the new one loads, then removes it (no unstyled flash)", () => { + retainPluginCss("hello"); applyPluginCss("hello", "/assets/app.css?h=aaa"); expect(links("hello")).toHaveLength(1); + links("hello")[0]?.dispatchEvent(new Event("load")); applyPluginCss("hello", "/assets/app.css?h=bbb"); // Both links coexist while the fresh sheet is still loading. @@ -843,7 +1049,9 @@ describe("applyPluginCss", () => { }); it("on load error, drops the new link and keeps the old sheet working", () => { + retainPluginCss("hello"); applyPluginCss("hello", "/assets/app.css?h=aaa"); + links("hello")[0]?.dispatchEvent(new Event("load")); applyPluginCss("hello", "/assets/app.css?h=bbb"); const fresh = links("hello")[1]; @@ -857,6 +1065,7 @@ describe("applyPluginCss", () => { }); it("keeps the same element for an unchanged URL and removes it on null", () => { + retainPluginCss("hello"); applyPluginCss("hello", "/assets/app.css?h=aaa"); const first = links("hello")[0]; applyPluginCss("hello", "/assets/app.css?h=aaa"); @@ -865,6 +1074,49 @@ describe("applyPluginCss", () => { applyPluginCss("hello", null); expect(links("hello")).toHaveLength(0); }); + + it("preloads inactive CSS and removes the sheet only after its final consumer releases", async () => { + applyPluginCss("hello", "/assets/app.css?h=aaa"); + expect(preloads("hello")).toHaveLength(1); + expect(preloads("hello")[0]?.fetchPriority).toBe("low"); + expect(links("hello")).toHaveLength(0); + + preloads("hello")[0]?.dispatchEvent(new Event("load")); + expect(preloads("hello")).toHaveLength(0); + const releaseFirst = retainPluginCss("hello"); + const releaseSecond = retainPluginCss("hello"); + expect(links("hello")).toHaveLength(1); + + releaseFirst(); + await Promise.resolve(); + expect(links("hello")).toHaveLength(1); + releaseSecond(); + await Promise.resolve(); + expect(links("hello")).toHaveLength(0); + }); + + it("never ties app-wide bb.themes palette CSS to plugin UI mounts", async () => { + const paletteCss = ":root { --canvas: rebeccapurple; }"; + applyAppThemeCss(paletteCss); + const palette = document.getElementById("bb-app-theme"); + expect(palette?.textContent).toBe(paletteCss); + + applyPluginCss("palette-owner", "/assets/app.css?h=palette-owner"); + preloads("palette-owner")[0]?.dispatchEvent(new Event("load")); + expect(links("palette-owner")).toHaveLength(0); + expect(document.getElementById("bb-app-theme")).toBe(palette); + expect(palette?.textContent).toBe(paletteCss); + + const release = retainPluginCss("palette-owner"); + release(); + await Promise.resolve(); + expect(links("palette-owner")).toHaveLength(0); + expect(document.getElementById("bb-app-theme")).toBe(palette); + expect(palette?.textContent).toBe(paletteCss); + + applyAppThemeCss(""); + palette?.remove(); + }); }); describe("createPluginFrontendReconcileScheduler", () => { diff --git a/apps/app/src/lib/plugin-frontend.ts b/apps/app/src/lib/plugin-frontend.ts index 0b1e2a2653..ac61e5a7c2 100644 --- a/apps/app/src/lib/plugin-frontend.ts +++ b/apps/app/src/lib/plugin-frontend.ts @@ -42,6 +42,7 @@ import type { import { normalizePluginThreadRowStatus } from "@get-bb/plugin-sdk/internal/composer-customization-validation"; import { resetCrashedPluginSlots } from "@/components/plugin/PluginSlotMount"; import { runWithPluginDomIsolationAsync } from "./foreign-dom-mutation-guard"; +import { applyPluginCss, retainPluginCss } from "./plugin-css"; import { collectPluginAppRegistrations, isPluginAppDefinition, @@ -346,69 +347,7 @@ async function fetchFrontendCandidates(): Promise { return candidates; } -/** - * Point a plugin's stylesheet `` at `url`, - * or remove it (`url: null`). A changed URL swaps in a fresh element (the - * new sheet loads, then the old element is removed) rather than mutating - * `href`, so a reload never flashes unstyled plugin UI. If the fresh sheet - * fails to load, it is dropped and the old sheet stays in place. - */ -export function applyPluginCss(pluginId: string, url: string | null): void { - const marker = "data-bb-plugin-css"; - const existing = [ - ...document.head.querySelectorAll(`link[${marker}="${pluginId}"]`), - ]; - if (url === null) { - for (const link of existing) link.remove(); - return; - } - if (existing.some((link) => link.getAttribute("href") === url)) return; - const link = document.createElement("link"); - link.rel = "stylesheet"; - link.href = url; - link.setAttribute(marker, pluginId); - link.onload = () => { - for (const old of existing) old.remove(); - }; - link.onerror = () => { - link.remove(); - console.warn(`bb plugin "${pluginId}": failed to load stylesheet ${url}`); - }; - document.head.appendChild(link); -} - -/** - * Coalesce plugin stylesheet insertions into one animation frame. Each - * `` append invalidates document-wide style; bundles - * finish importing at scattered moments, so without this every plugin costs - * its own recalc. Removals (`url: null`) stay synchronous — teardown and - * disposal must not leave a sheet behind — and cancel a pending insertion. - * A newer URL for the same plugin replaces the pending one. - */ -export function createBatchedPluginCssApplier(args: { - apply: (pluginId: string, url: string | null) => void; - requestFrame: (callback: () => void) => void; -}): (pluginId: string, url: string | null) => void { - const pending = new Map(); - let frameRequested = false; - const flush = () => { - frameRequested = false; - const batch = [...pending]; - pending.clear(); - for (const [pluginId, url] of batch) args.apply(pluginId, url); - }; - return (pluginId, url) => { - if (url === null) { - pending.delete(pluginId); - args.apply(pluginId, null); - return; - } - pending.set(pluginId, url); - if (frameRequested) return; - frameRequested = true; - args.requestFrame(flush); - }; -} +export { applyPluginCss } from "./plugin-css"; /** How many plugin bundles import at once during a reconcile pass. */ export const PLUGIN_FRONTEND_LOAD_CONCURRENCY = 3; @@ -485,8 +424,10 @@ export function createPluginFrontendReconcileState(): PluginFrontendReconcileSta export interface PluginFrontendReconcileDeps { fetchCandidates: () => Promise; importModule: (url: string) => Promise; - /** Replace (string) or remove (null) the plugin's CSS ``. */ + /** Synchronously publish (string) or remove (null) the generation's CSS URL. */ applyCss: (pluginId: string, url: string | null) => void; + /** Retain the published CSS through one non-React consumer's lifetime. */ + retainCss: (pluginId: string) => () => void; resetCrashedSlots: (pluginId: string) => void; setRegistrations: ( pluginId: string, @@ -521,6 +462,7 @@ interface ActivePluginFrontendGeneration { controller: AbortController; statusOwner: symbol; scripts: MountedContentScript[]; + cssRelease: (() => void) | null; disposed: boolean; } @@ -586,6 +528,8 @@ async function disposeGeneration( ); if (failure !== null) failures.push(failure); } + activation.cssRelease?.(); + activation.cssRelease = null; clearPluginThreadRowStatusesByOwner(activation.statusOwner); return failures; } @@ -594,6 +538,7 @@ async function deactivateCommittedGeneration( pluginId: string, state: PluginFrontendReconcileState, deps: PluginFrontendReconcileDeps, + removePublishedUi = true, ): Promise { const active = state.activeGenerations.get(pluginId); if (active === undefined) { @@ -604,8 +549,10 @@ async function deactivateCommittedGeneration( clearPluginThreadRowStatuses(pluginId); state.activeGenerations.delete(pluginId); state.appliedHashes.delete(pluginId); - deps.removeRegistrations(pluginId); - deps.applyCss(pluginId, null); + if (removePublishedUi) { + deps.removeRegistrations(pluginId); + deps.applyCss(pluginId, null); + } return failures; } @@ -708,6 +655,7 @@ async function activateContentScripts( registrations: readonly PluginContentScriptRegistration[], controller: AbortController, statusOwner: symbol, + cssRelease: (() => void) | null, deps: PluginFrontendReconcileDeps, ): Promise< | { ok: true; activation: ActivePluginFrontendGeneration } @@ -719,6 +667,7 @@ async function activateContentScripts( controller, statusOwner, scripts: [], + cssRelease, disposed: false, }; try { @@ -897,10 +846,20 @@ async function reconcileCandidates( const generation = (state.generationByPluginId.get(pluginId) ?? 0) + 1; state.generationByPluginId.set(pluginId, generation); + // Publish the URL before either non-React scripts mount or slot-store + // notifications can render plugin code. Inactive plugins only preload; + // an already-mounted generation starts a safe side-by-side replacement. + deps.applyCss(pluginId, candidate.bundle.cssUrl); + const cssRelease = + collected.contentScripts.length > 0 ? deps.retainCss(pluginId) : null; const disposeFailures = await deactivateCommittedGeneration( pluginId, state, deps, + // Keep the old registration mounted until candidate content scripts + // succeed. The final setRegistrations call replaces it atomically, so + // an old UI consumer holds the active sheet through the CSS handoff. + false, ); const controller = new AbortController(); const statusOwner = Symbol( @@ -915,6 +874,7 @@ async function reconcileCandidates( collected.contentScripts, controller, statusOwner, + cssRelease, deps, ); state.pendingControllers.delete(pluginId); @@ -923,9 +883,13 @@ async function reconcileCandidates( if (activationResult.ok) { await disposeGeneration(pluginId, activationResult.activation, deps); } + deps.removeRegistrations(pluginId); + deps.applyCss(pluginId, null); return; } if (!activationResult.ok) { + deps.removeRegistrations(pluginId); + deps.applyCss(pluginId, null); const failed: PluginFrontendRecord = { pluginId, status: "failed", @@ -943,7 +907,6 @@ async function reconcileCandidates( state.activeGenerations.set(pluginId, activationResult.activation); deps.setRegistrations(pluginId, collected); - deps.applyCss(pluginId, candidate.bundle.cssUrl); state.records.set(pluginId, record); state.appliedHashes.set(pluginId, candidate.bundle.hash); publishDiagnostic(state, deps, { @@ -1060,12 +1023,8 @@ const PLUGIN_SLOT_BATCH_MAX_HOLD_MS = 150; const browserReconcileDeps: PluginFrontendReconcileDeps = { fetchCandidates: fetchFrontendCandidates, importModule: (url) => import(/* @vite-ignore */ url), - applyCss: createBatchedPluginCssApplier({ - apply: applyPluginCss, - requestFrame: (callback) => { - window.requestAnimationFrame(callback); - }, - }), + applyCss: applyPluginCss, + retainCss: retainPluginCss, routePluginId: () => getPluginPanelRoutePluginId(window.location.pathname), resetCrashedSlots: resetCrashedPluginSlots, setRegistrations: setPluginSlotRegistrations, diff --git a/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md b/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md index 6ed8dc424e..679c9cb429 100644 --- a/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md +++ b/apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md @@ -1522,13 +1522,21 @@ window's last load/setup/mount/dispose failure appears on the plugin Settings detail page. The host cannot catch a detached promise that plugin code creates and never returns, so detached work must handle its own errors. -Prefer the existing imported `app.css` pipeline for static styles. A content -script may create DOM or `