diff --git a/end-to-end-tests/tests/runtime/modVariables/variableSync.spec.ts b/end-to-end-tests/tests/runtime/modVariables/variableSync.spec.ts new file mode 100644 index 000000000..0a5370827 --- /dev/null +++ b/end-to-end-tests/tests/runtime/modVariables/variableSync.spec.ts @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { test, expect } from "../../../fixtures/testBase"; +import { ActivateModPage } from "../../../pageObjects/extensionConsole/modsPage"; +// @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only +import { type Frame, type Page, test as base } from "@playwright/test"; +import { getSidebarPage } from "../../../utils"; + +test.describe("Mod Variable Sync", () => { + test("session variable cross-tab sync", async ({ page, extensionId }) => { + await test.step("activate mod", async () => { + const modId = "@e2e-testing/state-sync"; + const modActivationPage = new ActivateModPage(page, extensionId, modId); + await modActivationPage.goto(); + await modActivationPage.clickActivateAndWaitForModsPageRedirect(); + + await page.goto("/frames-builder.html"); + }); + + // Waiting for the mod to be ready before opening sidebar + await expect(page.getByText("Local: 0")).toBeVisible(); + + // The mod contains a trigger to open the sidebar on h1 + await page.click("h1"); + const sideBarPage = await getSidebarPage(page, extensionId); + await expect( + sideBarPage.getByRole("heading", { name: "State Sync Demo" }), + ).toBeVisible(); + + await test.step("verify same tab increment", async () => { + await sideBarPage.getByRole("button", { name: "Increment" }).click(); + + await expect(sideBarPage.getByText("Sync: 1")).toBeVisible(); + await expect(sideBarPage.getByText("Local: 1")).toBeVisible(); + + await expect(page.getByText("Sync: 1")).toBeVisible(); + await expect(page.getByText("Local: 1")).toBeVisible(); + + const frameLocator = page.frameLocator("iframe"); + await expect(frameLocator.getByText("Sync: 1")).toBeVisible(); + await expect(frameLocator.getByText("Local: 0")).toBeVisible(); + }); + + // Close the sidebar, because getSidebarPage currently can't distinguish between multiple sidebars + await sideBarPage.getByRole("button", { name: "Close" }).click(); + await sideBarPage.getByRole("button", { name: "Close" }).click(); + + const otherPage = await page.context().newPage(); + await otherPage.goto(page.url()); + + // Waiting for the mod to be ready before opening sidebar + await expect(otherPage.getByText("Local: 0")).toBeVisible(); + + await otherPage.click("h1"); + const otherSideBarPage = await getSidebarPage(otherPage, extensionId); + await expect( + otherSideBarPage.getByRole("heading", { name: "State Sync Demo" }), + ).toBeVisible(); + + await test.step("verify cross tab increment", async () => { + // Should be available on first run of the panel + await expect(otherSideBarPage.getByText("Sync: 1")).toBeVisible(); + await expect(otherSideBarPage.getByText("Local: ")).toBeVisible(); + + await otherSideBarPage.getByRole("button", { name: "Increment" }).click(); + + await expect(otherSideBarPage.getByText("Sync: 2")).toBeVisible(); + await expect(otherSideBarPage.getByText("Local: 1")).toBeVisible(); + + // Should automatically sync to the original tab + await expect(page.getByText("Sync: 2")).toBeVisible(); + await expect(page.getByText("Local: 1")).toBeVisible(); + + const frameLocator = page.frameLocator("iframe"); + await expect(frameLocator.getByText("Sync: 2")).toBeVisible(); + // Local variable doesn't exist in the frame + await expect(frameLocator.getByText("Local: 0")).toBeVisible(); + }); + }); +}); diff --git a/end-to-end-tests/utils.ts b/end-to-end-tests/utils.ts index 6a94c3673..5a0873f4c 100644 --- a/end-to-end-tests/utils.ts +++ b/end-to-end-tests/utils.ts @@ -90,12 +90,18 @@ export async function runModViaQuickBar(page: Page, modName: string) { } function findSidebarPage(page: Page, extensionId: string): Page | undefined { - return page + const matches = page .context() .pages() - .find((value) => + .filter((value) => value.url().startsWith(`chrome-extension://${extensionId}/sidebar.html`), ); + + if (matches.length > 1) { + throw new Error("Multiple sidebar pages found"); + } + + return matches[0]; } /** @@ -111,10 +117,12 @@ export function isSidebarOpen(page: Page, extensionId: string): boolean { /** * Finds the Pixiebrix sidebar page/frame. * + * NOTE: returns the sidebar found, not necessarily the one for the provided page. + * * Automatically clicks "OK" on the dialog that appears if the sidebar requires a user gesture to open * This is a Page contained in the browser sidepanel window. * - * @throws {Error} if the sidebar is not available + * @throws {Error} if the sidebar is not available or multiple pages have the sidebar open */ export async function getSidebarPage( page: Page, diff --git a/src/background/background.ts b/src/background/background.ts index daa3b6631..708e8fe87 100644 --- a/src/background/background.ts +++ b/src/background/background.ts @@ -55,6 +55,14 @@ import initTeamTrialUpdater from "@/background/teamTrialUpdater"; // In the future, it might also run other background tasks from mods (e.g., background intervals) setPlatform(backgroundPlatform); +// Allows the content script to directly access the session storage for mod variables. Without this, we'd need to +// we'd need to use the messenger or localStorage to synchronize state across frames. +// Does not trigger a permissions prompt, see: +// https://developer.chrome.com/docs/extensions/reference/api/storage#type-AccessLevel +void chrome.storage.session.setAccessLevel({ + accessLevel: "TRUSTED_AND_UNTRUSTED_CONTEXTS", +}); + void initLocator(); void initMessengerLogging(); void initRuntimeLogging(); diff --git a/src/contentScript/contentScriptPlatform.ts b/src/contentScript/contentScriptPlatform.ts index b267cf057..17dc89d04 100644 --- a/src/contentScript/contentScriptPlatform.ts +++ b/src/contentScript/contentScriptPlatform.ts @@ -27,7 +27,11 @@ import { traces, uninstallContextMenu, } from "@/background/messenger/api"; -import { getState, setState } from "@/contentScript/stateController"; +import { + getState, + registerModVariables, + setState, +} from "@/contentScript/stateController"; import quickBarRegistry from "@/components/quickBar/quickBarRegistry"; import { expectContext } from "@/utils/expectContext"; import type { PlatformCapability } from "@/platform/capabilities"; @@ -242,6 +246,7 @@ class ContentScriptPlatform extends PlatformBase { return { getState, setState, + registerModVariables, }; } diff --git a/src/contentScript/stateController.test.ts b/src/contentScript/stateController.test.ts index aa1b5fc56..444c7622a 100644 --- a/src/contentScript/stateController.test.ts +++ b/src/contentScript/stateController.test.ts @@ -15,13 +15,18 @@ * along with this program. If not, see . */ -import { setState } from "@/contentScript/stateController"; +import { + setState, + registerModVariables, + getState, +} from "@/contentScript/stateController"; import { modComponentRefFactory } from "@/testUtils/factories/modComponentFactories"; import { MergeStrategies, STATE_CHANGE_JS_EVENT_TYPE, StateNamespaces, } from "@/platform/state/stateTypes"; +import type { JSONSchema7Definition } from "json-schema"; describe("pageState", () => { it("deep merge triggers event", async () => { @@ -70,4 +75,55 @@ describe("pageState", () => { asyncState: { isFetching: true, currentData: null, data: "foo" }, }); }); + + describe("mod variable policy", () => { + it("stores variable in session storage", async () => { + const listener = jest.fn(); + + document.addEventListener(STATE_CHANGE_JS_EVENT_TYPE, listener); + + const modComponentRef = modComponentRefFactory(); + + await expect(browser.storage.session.get(null)).resolves.toStrictEqual( + {}, + ); + + registerModVariables(modComponentRef.modId, { + schema: { + type: "object", + properties: { + foo: { + type: "object", + "x-sync-policy": "session", + // Cast required because types don't support custom `x-` variables yet + } as JSONSchema7Definition, + }, + required: ["foo"], + }, + }); + + await setState({ + namespace: StateNamespaces.MOD, + data: { foo: { bar: "baz" }, quox: 42 }, + mergeStrategy: MergeStrategies.REPLACE, + modComponentRef, + }); + + // The storage fake doesn't emit events + expect(listener).toHaveBeenCalledTimes(0); + + const values = await browser.storage.session.get(null); + + // Ensure values are segmented correctly in storage + expect(JSON.stringify(values)).not.toContain("quox"); + expect(JSON.stringify(values)).toContain("bar"); + + const state = await getState({ + namespace: StateNamespaces.MOD, + modComponentRef, + }); + + expect(state).toEqual({ foo: { bar: "baz" }, quox: 42 }); + }); + }); }); diff --git a/src/contentScript/stateController.ts b/src/contentScript/stateController.ts index e1f1cc7e3..012807b2d 100644 --- a/src/contentScript/stateController.ts +++ b/src/contentScript/stateController.ts @@ -17,11 +17,11 @@ import { type UUID } from "@/types/stringTypes"; import { type RegistryId } from "@/types/registryTypes"; -import { cloneDeep, isEqual, merge } from "lodash"; +import { cloneDeep, isEmpty, isEqual, merge, omit, pick } from "lodash"; import { BusinessError } from "@/errors/businessErrors"; import { type Except, type JsonObject } from "type-fest"; import { assertPlatformCapability } from "@/platform/platformContext"; -import { assertNotNullish } from "@/utils/nullishUtils"; +import { assertNotNullish, type Nullishable } from "@/utils/nullishUtils"; import { type ModComponentRef } from "@/types/modComponentTypes"; import { MergeStrategies, @@ -30,16 +30,89 @@ import { type StateChangeEventDetail, type StateNamespace, StateNamespaces, + SyncPolicies, + type SyncPolicy, } from "@/platform/state/stateTypes"; +import { type ModVariablesDefinition } from "@/types/modDefinitionTypes"; +import { validateRegistryId } from "@/types/helpers"; +import { Storage } from "webextension-polyfill"; +import StorageChange = Storage.StorageChange; +import { emptyModVariablesDefinitionFactory } from "@/utils/modUtils"; -// eslint-disable-next-line local-rules/persistBackgroundData -- content script -const privateState = new Map(); +// The exact prefix is not important. Pick one that is unlikely to collide with other keys. +const keyPrefix = "#modVariables/"; + +const SCHEMA_POLICY_PROP = "x-sync-policy"; + +/** + * Map from mod variable name to its synchronization policy. + * Excludes variables with SyncPolicies.NONE. + */ +type VariableSyncPolicyMapping = Record; + +/** + * Map from mod component id to its private state. + */ +// eslint-disable-next-line local-rules/persistBackgroundData -- content script state +const framePrivateState = new Map(); + +/** + * Map from mod id to its mod state. Or null key for public page state. + */ +// eslint-disable-next-line local-rules/persistBackgroundData -- content script state +const frameModState = new Map(); + +/** + * Map from mod id to its variable synchronization policy. + * @since 2.1.3 + */ +// eslint-disable-next-line local-rules/persistBackgroundData -- content script state +const modSyncPolicies = new Map(); + +function getSessionStorageKey(modId: RegistryId): string { + return `${keyPrefix}${modId}`; +} /** - * The mod page state or null for shared page state. + * Returns the current state of the mod variables according to the mod's variable synchronization policy. */ -// eslint-disable-next-line local-rules/persistBackgroundData -- content script -const modState = new Map(); +async function getModVariableState(modId: RegistryId): Promise { + const modPolicy = modSyncPolicies.get(modId); + const syncedVariableNames = Object.keys(modPolicy ?? {}); + + let synced = {}; + + if (!isEmpty(modPolicy)) { + const key = getSessionStorageKey(modId); + // Skip call if there are no synchronized variables + const value = await browser.storage.session.get(key); + // eslint-disable-next-line security/detect-object-injection -- key passed to .get + synced = value[key] ?? {}; + } + + const local = frameModState.get(modId) ?? {}; + + return { + ...omit(local, syncedVariableNames), + ...pick(synced, syncedVariableNames), + }; +} + +async function updateModVariableState( + modId: RegistryId, + nextState: JsonObject, +): Promise { + const modPolicy = modSyncPolicies.get(modId); + const syncedVariableNames = Object.keys(modPolicy ?? {}); + + frameModState.set(modId, omit(nextState, syncedVariableNames)); + + if (!isEmpty(modPolicy)) { + const key = getSessionStorageKey(modId); + const synced = pick(nextState, syncedVariableNames); + await browser.storage.session.set({ [key]: synced }); + } +} function mergeState( previous: JsonObject, @@ -69,33 +142,55 @@ function mergeState( } } +/** + * Dispatch a state change event to the document. + * @see STATE_CHANGE_JS_EVENT_TYPE + */ +function dispatchStateChangeEvent( + // For now, leave off the state data because state controller in the content script uses JavaScript/DOM + // events, which is a public channel (the host site/other extensions can see the event). + detail: StateChangeEventDetail, +) { + console.debug("Dispatching statechange", detail); + + document.dispatchEvent( + new CustomEvent(STATE_CHANGE_JS_EVENT_TYPE, { + detail, + bubbles: true, + }), + ); +} + function dispatchStateChangeEventOnChange({ previous, next, namespace, modComponentRef: { modComponentId, modId }, }: { - previous: unknown; - next: unknown; + previous: JsonObject; + next: JsonObject; namespace: StateNamespace; modComponentRef: Except; }) { + const modPolicy = modSyncPolicies.get(modId); + const syncedVariableNames = Object.keys(modPolicy ?? {}); + + if ( + !isEqual( + pick(previous, syncedVariableNames), + pick(next, syncedVariableNames), + ) + ) { + // Skip firing because it will be fired by the session storage listener + return; + } + if (!isEqual(previous, next)) { - // For now, leave off the event data because state controller in the content script uses JavaScript/DOM - // events, which is a public channel (the host site/other extensions can see the event). - const detail = { + dispatchStateChangeEvent({ namespace, extensionId: modComponentId, blueprintId: modId, - } satisfies StateChangeEventDetail; - - console.debug("Dispatching statechange", detail); - - const event = new CustomEvent(STATE_CHANGE_JS_EVENT_TYPE, { - detail, - bubbles: true, }); - document.dispatchEvent(event); } } @@ -125,17 +220,17 @@ export async function setState({ switch (namespace) { case StateNamespaces.PUBLIC: { - const previous = modState.get(null) ?? {}; + const previous = frameModState.get(null) ?? {}; const next = mergeState(previous, data, mergeStrategy); - modState.set(null, next); + frameModState.set(null, next); notifyOnChange(previous, next); return next; } case StateNamespaces.MOD: { - const previous = modState.get(modId) ?? {}; + const previous = await getModVariableState(modId); const next = mergeState(previous, data, mergeStrategy); - modState.set(modId, next); + await updateModVariableState(modId, next); notifyOnChange(previous, next); return next; } @@ -145,9 +240,9 @@ export async function setState({ modComponentId, "Invalid context: mod component id not found", ); - const previous = privateState.get(modComponentId) ?? {}; + const previous = framePrivateState.get(modComponentId) ?? {}; const next = mergeState(previous, data, mergeStrategy); - privateState.set(modComponentId, next); + framePrivateState.set(modComponentId, next); notifyOnChange(previous, next); return next; } @@ -170,11 +265,11 @@ export async function getState({ switch (namespace) { case StateNamespaces.PUBLIC: { - return modState.get(null) ?? {}; + return frameModState.get(null) ?? {}; } case StateNamespaces.MOD: { - return modState.get(modId) ?? {}; + return getModVariableState(modId); } case StateNamespaces.PRIVATE: { @@ -182,7 +277,7 @@ export async function getState({ modComponentId, "Invalid context: mod component id not found", ); - return privateState.get(modComponentId) ?? {}; + return framePrivateState.get(modComponentId) ?? {}; } default: { @@ -192,7 +287,74 @@ export async function getState({ } } +function mapModVariablesToModSyncPolicy( + variables: ModVariablesDefinition, +): VariableSyncPolicyMapping { + return Object.fromEntries( + Object.entries(variables.schema.properties ?? {}) + .map(([key, definition]) => { + // eslint-disable-next-line security/detect-object-injection -- constant + const variablePolicy = (definition as UnknownObject)[ + SCHEMA_POLICY_PROP + ] as SyncPolicy | undefined; + + if (variablePolicy && variablePolicy !== SyncPolicies.NONE) { + if (variablePolicy !== SyncPolicies.SESSION) { + throw new BusinessError( + `Unsupported sync policy: ${variablePolicy}`, + ); + } + + return [key, variablePolicy] satisfies [string, SyncPolicy]; + } + + return null; + }) + .filter((x) => x != null), + ); +} + +// Keep as separate method so it's safe to call addListener multiple times with the listener +function onSessionStorageChange( + change: Record, + areaName: string, +): void { + if (areaName === "session") { + for (const key of Object.keys(change)) { + if (key.startsWith(keyPrefix)) { + dispatchStateChangeEvent({ + namespace: StateNamespaces.MOD, + blueprintId: validateRegistryId(key.slice(keyPrefix.length)), + }); + } + } + } +} + +/** + * Register variables and their synchronization policy for a mod. + * @param modId the mod registry id + * @param variables the mod variables definition containing their synchronization policy. If nullish, a blank policy + * is registered. + * @see emptyModVariablesDefinitionFactory + */ +export function registerModVariables( + modId: RegistryId, + variables: Nullishable, +): void { + const modSyncPolicy = mapModVariablesToModSyncPolicy( + variables ?? emptyModVariablesDefinitionFactory(), + ); + modSyncPolicies.set(modId, modSyncPolicy); + + // If any variables are set to sync, listen for changes to session storage to notify the mods running on this page + if (!isEmpty(modSyncPolicy)) { + browser.storage.onChanged.addListener(onSessionStorageChange); + } +} + export function TEST_resetState(): void { - privateState.clear(); - modState.clear(); + framePrivateState.clear(); + frameModState.clear(); + modSyncPolicies.clear(); } diff --git a/src/platform/platformTypes/stateProtocol.ts b/src/platform/platformTypes/stateProtocol.ts index f0f4d7e64..435226ed7 100644 --- a/src/platform/platformTypes/stateProtocol.ts +++ b/src/platform/platformTypes/stateProtocol.ts @@ -21,6 +21,9 @@ import type { } from "@/platform/state/stateTypes"; import type { Except, JsonObject } from "type-fest"; import type { ModComponentRef } from "@/types/modComponentTypes"; +import type { RegistryId } from "@/types/registryTypes"; +import type { ModVariablesDefinition } from "@/types/modDefinitionTypes"; +import type { Nullishable } from "@/utils/nullishUtils"; /** * The variable store/state for the platform. @@ -51,4 +54,19 @@ export type StateProtocol = { data: JsonObject; mergeStrategy: MergeStrategy; }): Promise; + + /** + * Register variables and their synchronization policy for a mod. + * + * Mods can write to variable names dynamically, but declaring variables supports automatic synchronization across + * tabs/frames, and better development support (e.g., type checking, descriptions, etc.) + * + * @param modId the mod id + * @param variables mod variables definition, or nullishable to allow any mod variable + * @since 2.1.3 + */ + registerModVariables( + modId: RegistryId, + variables: Nullishable, + ): void; }; diff --git a/src/platform/state/stateTypes.ts b/src/platform/state/stateTypes.ts index ebdbffd6b..d3cc66c30 100644 --- a/src/platform/state/stateTypes.ts +++ b/src/platform/state/stateTypes.ts @@ -17,7 +17,6 @@ import type { ValueOf } from "type-fest"; import type { UUID } from "@/types/stringTypes"; -import type { Nullishable } from "@/utils/nullishUtils"; import type { RegistryId } from "@/types/registryTypes"; export const MergeStrategies = { @@ -36,6 +35,17 @@ export const StateNamespaces = { export type StateNamespace = ValueOf; +/** + * [Experimental] policy to sync state changes across frames/tabs. + * @since 2.1.3 + */ +export const SyncPolicies = { + NONE: "none", + SESSION: "session", +}; + +export type SyncPolicy = ValueOf; + /** * JavaScript event name fired for state change events. */ @@ -49,8 +59,8 @@ export type StateChangeEventDetail = { // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/8845 -- rename deprecated fieldnames // Keep using extensionId/blueprintId for now for backward compatability because the values are made available // in `@input.event`. It's unlikely anyone is relying on them in the wild, though. - extensionId: UUID; - blueprintId: Nullishable; + blueprintId: RegistryId; + extensionId?: UUID; }; /** diff --git a/src/starterBricks/types.ts b/src/starterBricks/types.ts index 9a6bd0662..1c87f6a1f 100644 --- a/src/starterBricks/types.ts +++ b/src/starterBricks/types.ts @@ -244,23 +244,36 @@ export abstract class StarterBrickABC ): void; synchronizeModComponents( - components: Array>, + modComponents: Array>, ): void { const before = this.modComponents.map((x) => x.id); - const updatedIds = new Set(components.map((x) => x.id)); + const updatedIds = new Set(modComponents.map((x) => x.id)); const removed = this.modComponents.filter( (currentComponent) => !updatedIds.has(currentComponent.id), ); this.clearModComponentInterfaceAndEvents(removed.map((x) => x.id)); - // Clear extensions and re-populate with updated components + // Clear this.modComponents and re-populate with updated components this.modComponents.length = 0; - this.modComponents.push(...components); + this.modComponents.push(...modComponents); + + // `registerModVariables` is safe to call multiple times for the same modId because the variable definitions + // will be consistent across components. + for (const modComponent of modComponents) { + if (modComponent._recipe) { + // `_recipe` is still optional on the type, but should always be present now that internal ids are generated + // for draft mod components. However, there's old test code that doesn't set `_recipe` on the mod component. + this.platform.state.registerModVariables( + modComponent._recipe.id, + modComponent.variablesDefinition, + ); + } + } console.debug("synchronizeComponents for extension point %s", this.id, { before, - after: components.map((x) => x.id), + after: modComponents.map((x) => x.id), removed: removed.map((x) => x.id), }); } @@ -271,17 +284,26 @@ export abstract class StarterBrickABC ); } - registerModComponent(component: HydratedModComponent): void { - const index = this.modComponents.findIndex((x) => x.id === component.id); + registerModComponent(modComponent: HydratedModComponent): void { + const index = this.modComponents.findIndex((x) => x.id === modComponent.id); if (index >= 0) { console.warn( - `Component ${component.id} already registered for the starter brick ${this.id}`, + `Component ${modComponent.id} already registered for the starter brick ${this.id}`, ); /* eslint-disable-next-line security/detect-object-injection -- -- Index is guaranteed to be a number, and this.modComponents is an array */ - this.modComponents[index] = component; + this.modComponents[index] = modComponent; } else { - this.modComponents.push(component); + this.modComponents.push(modComponent); + } + + if (modComponent._recipe) { + // `_recipe` is still optional on the type, but should always be present now that internal ids are generated + // for draft mod components. However, there's old test code that doesn't set `_recipe` on the mod component. + this.platform.state.registerModVariables( + modComponent._recipe.id, + modComponent.variablesDefinition, + ); } } diff --git a/src/testUtils/platformMock.ts b/src/testUtils/platformMock.ts index f2ed6674a..5b2cb983c 100644 --- a/src/testUtils/platformMock.ts +++ b/src/testUtils/platformMock.ts @@ -56,6 +56,7 @@ export const platformMock: PlatformProtocol = { state: { getState: jest.fn(), setState: jest.fn(), + registerModVariables: jest.fn(), }, templates: { render: jest.fn(),