From 46d2207ad7e2170010cedaf238adcb1f802c1daa Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 15:31:50 +0700 Subject: [PATCH 01/33] POC: Isolated components --- src/components/EmotionShadowRoot.ts | 2 +- src/isolatedComponents/C.module.scss | 3 ++ src/isolatedComponents/C.scss | 3 ++ src/isolatedComponents/IsolatedComponent.tsx | 56 ++++++++++++++++++++ src/isolatedComponents/Window.tsx | 13 +++++ src/utils/notify.tsx | 16 +++++- webpack.config.mjs | 1 + 7 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/isolatedComponents/C.module.scss create mode 100644 src/isolatedComponents/C.scss create mode 100644 src/isolatedComponents/IsolatedComponent.tsx create mode 100644 src/isolatedComponents/Window.tsx diff --git a/src/components/EmotionShadowRoot.ts b/src/components/EmotionShadowRoot.ts index f020f85a1d..27f11e2746 100644 --- a/src/components/EmotionShadowRoot.ts +++ b/src/components/EmotionShadowRoot.ts @@ -22,7 +22,7 @@ import { type CSSProperties } from "react"; /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- "Every property exists" (via Proxy), TypeScript doesn't offer such type Also strictNullChecks config mismatch */ -const ShadowRoot = EmotionShadowRoot.div!; +const ShadowRoot = EmotionShadowRoot["pixiebrix-widget"]!; export const styleReset: CSSProperties = { all: "initial", diff --git a/src/isolatedComponents/C.module.scss b/src/isolatedComponents/C.module.scss new file mode 100644 index 0000000000..d810e45683 --- /dev/null +++ b/src/isolatedComponents/C.module.scss @@ -0,0 +1,3 @@ +.root { + color: red; +} diff --git a/src/isolatedComponents/C.scss b/src/isolatedComponents/C.scss new file mode 100644 index 0000000000..d8ccc9d491 --- /dev/null +++ b/src/isolatedComponents/C.scss @@ -0,0 +1,3 @@ +* { + color: red; +} diff --git a/src/isolatedComponents/IsolatedComponent.tsx b/src/isolatedComponents/IsolatedComponent.tsx new file mode 100644 index 0000000000..1fb77c8e0c --- /dev/null +++ b/src/isolatedComponents/IsolatedComponent.tsx @@ -0,0 +1,56 @@ +/* + * 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 React, { Suspense, useEffect } from "react"; +import { Stylesheets } from "@/components/Stylesheets"; +import EmotionShadowRoot, { styleReset } from "@/components/EmotionShadowRoot"; + +const MODE = process.env.SHADOW_DOM as "open" | "closed"; + +export const IsolatedComponent: React.VFC<{ + LazyComponent: React.LazyExoticComponent; + webpackChunkName: string; + className?: string; +}> = ({ webpackChunkName, LazyComponent, ...props }) => { + const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); + useEffect(() => { + const observer = new MutationObserver(([mutations]) => { + const link = document.head.querySelector( + `link[href="${stylesheetUrl}"]`, + ); + if (link) { + // Disable stylesheet without removing it. Webpack still awaits its loading. + link.media = "not all"; + observer.disconnect(); + } + }); + observer.observe(document.head, { childList: true }); + return () => { + observer.disconnect(); + }; + }); + + return ( + + + + + + + + ); +}; diff --git a/src/isolatedComponents/Window.tsx b/src/isolatedComponents/Window.tsx new file mode 100644 index 0000000000..0529fc7e57 --- /dev/null +++ b/src/isolatedComponents/Window.tsx @@ -0,0 +1,13 @@ +import "./C.scss"; +import React from "react"; +import styles from "./C.module.scss"; + +const Window: React.FC = () => ( +
+

C

+

hello

+ text +
+); + +export default Window; diff --git a/src/utils/notify.tsx b/src/utils/notify.tsx index 93d1c89bf3..77593de39c 100644 --- a/src/utils/notify.tsx +++ b/src/utils/notify.tsx @@ -17,7 +17,7 @@ import styles from "./notify.module.scss"; -import React from "react"; +import React, { lazy } from "react"; import { render } from "react-dom"; import { type DefaultToastOptions, @@ -40,6 +40,7 @@ import { SIDEBAR_WIDTH_CSS_PROPERTY } from "@/contentScript/sidebarDomController import ErrorIcon from "@/icons/error.svg?loadAsComponent"; import WarningIcon from "@/icons/warning.svg?loadAsComponent"; import type { Notification, NotificationType } from "@/utils/notificationTypes"; +import { IsolatedComponent } from "@/isolatedComponents/IsolatedComponent"; const MINIMUM_NOTIFICATION_DURATION_MS = 2000; @@ -126,6 +127,19 @@ export function initToaster(): void { error: toastStyle.error, }; render(, root); + + const LazyButton = lazy( + async () => + import(/* webpackChunkName: "Window" */ "@/isolatedComponents/Window"), + ); + + render( + , + document.querySelector("ul"), + ); } export function showNotification({ diff --git a/webpack.config.mjs b/webpack.config.mjs index 355e3beac1..217515075d 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -255,6 +255,7 @@ const createConfig = (env, options) => DEV_EVENT_TELEMETRY: false, SANDBOX_LOGGING: false, IS_BETA: process.env.PUBLIC_NAME === "-beta", + SHADOW_DOM: "closed", // If not found, "undefined" will cause the build to fail SERVICE_URL: undefined, From 708ba2b2cc0401196c7700270ebadee712ada7dd Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 17:17:39 +0700 Subject: [PATCH 02/33] Wrap `SelectionMenu` --- src/components/EmotionShadowRoot.ts | 12 +++-- .../IsolatedComponent.tsx | 50 ++++++++++++++++--- src/components/quickBar/QuickBarApp.tsx | 6 --- .../textSelectionMenu/SelectionMenu.tsx | 40 ++++++--------- .../selectionMenuController.tsx | 21 +++++--- src/isolatedComponents/C.module.scss | 3 -- src/isolatedComponents/C.scss | 3 -- src/isolatedComponents/Window.tsx | 13 ----- src/utils/notify.tsx | 16 +----- 9 files changed, 84 insertions(+), 80 deletions(-) rename src/{isolatedComponents => components}/IsolatedComponent.tsx (61%) delete mode 100644 src/isolatedComponents/C.module.scss delete mode 100644 src/isolatedComponents/C.scss delete mode 100644 src/isolatedComponents/Window.tsx diff --git a/src/components/EmotionShadowRoot.ts b/src/components/EmotionShadowRoot.ts index 27f11e2746..9042748165 100644 --- a/src/components/EmotionShadowRoot.ts +++ b/src/components/EmotionShadowRoot.ts @@ -1,3 +1,7 @@ +/* +eslint-disable @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- +"Every property exists" (via Proxy), TypeScript doesn't offer such type +Also strictNullChecks config mismatch */ /* * Copyright (C) 2024 PixieBrix, Inc. * @@ -19,9 +23,11 @@ import EmotionShadowRoot from "react-shadow/emotion"; import { type CSSProperties } from "react"; -/* eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- -"Every property exists" (via Proxy), TypeScript doesn't offer such type -Also strictNullChecks config mismatch */ +/** + * Wrap components in a shadow DOM. This isolates them from styles inherited from + * the host website. To support react-select and any future potential emotion + * components we used the emotion variant of the react-shadow library. + */ const ShadowRoot = EmotionShadowRoot["pixiebrix-widget"]!; export const styleReset: CSSProperties = { diff --git a/src/isolatedComponents/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx similarity index 61% rename from src/isolatedComponents/IsolatedComponent.tsx rename to src/components/IsolatedComponent.tsx index 1fb77c8e0c..75abc31d33 100644 --- a/src/isolatedComponents/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -17,15 +17,52 @@ import React, { Suspense, useEffect } from "react"; import { Stylesheets } from "@/components/Stylesheets"; -import EmotionShadowRoot, { styleReset } from "@/components/EmotionShadowRoot"; +import EmotionShadowRoot from "@/components/EmotionShadowRoot"; +import { css } from "code-tag"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; +const styleReset = css` + :host { + all: initial; + font: 16px / 1.5 sans-serif; + color-scheme: light; + } + :host::selection { + background: initial; + } +`; + +/** + * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. + * + * @example + * const Component = React.lazy(() => import( + * /* webpackChunkName: Component / + * "./Component" + * )); + * + * render( + * , + * document.querySelector('#container') + * ) + */ export const IsolatedComponent: React.VFC<{ - LazyComponent: React.LazyExoticComponent; + /** + * It must match the `webpackChunkName` specified in the React.lazy import + */ webpackChunkName: string; + + /** + * It must be the result of React.lazy() + */ + children: JSX.Element; + className?: string; -}> = ({ webpackChunkName, LazyComponent, ...props }) => { +}> = ({ webpackChunkName, children, ...props }) => { const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); useEffect(() => { const observer = new MutationObserver(([mutations]) => { @@ -45,11 +82,10 @@ export const IsolatedComponent: React.VFC<{ }); return ( - + + - - - + {children} ); diff --git a/src/components/quickBar/QuickBarApp.tsx b/src/components/quickBar/QuickBarApp.tsx index 70c4abf84c..069de21ed7 100644 --- a/src/components/quickBar/QuickBarApp.tsx +++ b/src/components/quickBar/QuickBarApp.tsx @@ -135,12 +135,6 @@ const KBarComponent: React.FC = () => { }} > - {/* - Wrap the quickbar in a shadow dom. This isolates the quickbar from styles being passed down from - whichever website it's rendering on. - To support react-select and any future potential emotion components we used the - emotion variant of the react-shadow library. - */} . */ +import "@/contentScript/textSelectionMenu/SelectionMenu.scss"; import React from "react"; -// We're rendering in the shadow DOM, so we need to load styles as a URL. loadAsUrl doesn't work with module mangling -import stylesUrl from "@/contentScript/textSelectionMenu/SelectionMenu.scss?loadAsUrl"; import Icon from "@/icons/Icon"; import { splitStartingEmoji } from "@/utils/stringUtils"; import { truncate } from "lodash"; import useDocumentSelection from "@/hooks/useDocumentSelection"; import type { Nullishable } from "@/utils/nullishUtils"; -import { Stylesheets } from "@/components/Stylesheets"; -import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import { getSelection } from "@/utils/selectionController"; import { type RegisteredAction } from "@/contentScript/textSelectionMenu/ActionRegistry"; import type ActionRegistry from "@/contentScript/textSelectionMenu/ActionRegistry"; @@ -94,26 +91,21 @@ const SelectionMenu: React.FC< // https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/menu_role return ( - // Prevent page styles from leaking into the menu - - -
- {[...actions.entries()].map(([id, action]) => ( - - ))} -
- - +
+ {[...actions.entries()].map(([id, action]) => ( + + ))} +
); }; diff --git a/src/contentScript/textSelectionMenu/selectionMenuController.tsx b/src/contentScript/textSelectionMenu/selectionMenuController.tsx index e8cb149d8c..9c13b8f3aa 100644 --- a/src/contentScript/textSelectionMenu/selectionMenuController.tsx +++ b/src/contentScript/textSelectionMenu/selectionMenuController.tsx @@ -30,17 +30,16 @@ import { type VirtualElement, } from "@floating-ui/dom"; import { getCaretCoordinates } from "@/utils/textAreaUtils"; -import TextSelectionMenu from "@/contentScript/textSelectionMenu/SelectionMenu"; import { expectContext } from "@/utils/expectContext"; import { onContextInvalidated } from "webext-events"; import { isNativeField } from "@/types/inputTypes"; import { onAbort, ReusableAbortController } from "abort-utils"; import { prefersReducedMotion } from "@/utils/a11yUtils"; import { getSelectionRange } from "@/utils/domUtils"; - import { snapWithin } from "@/utils/mathUtils"; import ActionRegistry from "@/contentScript/textSelectionMenu/ActionRegistry"; import { SELECTION_MENU_READY_ATTRIBUTE } from "@/domConstants"; +import { IsolatedComponent } from "@/components/IsolatedComponent"; const MIN_SELECTION_LENGTH_CHARS = 3; @@ -165,11 +164,21 @@ function createSelectionMenu(): HTMLElement { selectionMenu = tooltipFactory(); selectionMenu.dataset.testid = "pixiebrix-selection-menu"; + const TextSelectionMenu = React.lazy( + async () => + import( + /* webpackChunkName: "SelectionMenu" */ + "@/contentScript/textSelectionMenu/SelectionMenu" + ), + ); + render( - , + + + , selectionMenu, ); diff --git a/src/isolatedComponents/C.module.scss b/src/isolatedComponents/C.module.scss deleted file mode 100644 index d810e45683..0000000000 --- a/src/isolatedComponents/C.module.scss +++ /dev/null @@ -1,3 +0,0 @@ -.root { - color: red; -} diff --git a/src/isolatedComponents/C.scss b/src/isolatedComponents/C.scss deleted file mode 100644 index d8ccc9d491..0000000000 --- a/src/isolatedComponents/C.scss +++ /dev/null @@ -1,3 +0,0 @@ -* { - color: red; -} diff --git a/src/isolatedComponents/Window.tsx b/src/isolatedComponents/Window.tsx deleted file mode 100644 index 0529fc7e57..0000000000 --- a/src/isolatedComponents/Window.tsx +++ /dev/null @@ -1,13 +0,0 @@ -import "./C.scss"; -import React from "react"; -import styles from "./C.module.scss"; - -const Window: React.FC = () => ( -
-

C

-

hello

- text -
-); - -export default Window; diff --git a/src/utils/notify.tsx b/src/utils/notify.tsx index 77593de39c..93d1c89bf3 100644 --- a/src/utils/notify.tsx +++ b/src/utils/notify.tsx @@ -17,7 +17,7 @@ import styles from "./notify.module.scss"; -import React, { lazy } from "react"; +import React from "react"; import { render } from "react-dom"; import { type DefaultToastOptions, @@ -40,7 +40,6 @@ import { SIDEBAR_WIDTH_CSS_PROPERTY } from "@/contentScript/sidebarDomController import ErrorIcon from "@/icons/error.svg?loadAsComponent"; import WarningIcon from "@/icons/warning.svg?loadAsComponent"; import type { Notification, NotificationType } from "@/utils/notificationTypes"; -import { IsolatedComponent } from "@/isolatedComponents/IsolatedComponent"; const MINIMUM_NOTIFICATION_DURATION_MS = 2000; @@ -127,19 +126,6 @@ export function initToaster(): void { error: toastStyle.error, }; render(, root); - - const LazyButton = lazy( - async () => - import(/* webpackChunkName: "Window" */ "@/isolatedComponents/Window"), - ); - - render( - , - document.querySelector("ul"), - ); } export function showNotification({ From 06715745f4694ea8e5a4ab72005e8f0e8a2f800b Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 18:31:11 +0700 Subject: [PATCH 03/33] Extract CSS, use regular element for now --- src/components/EmotionShadowRoot.ts | 3 ++- src/components/IsolatedComponent.scss | 35 +++++++++++++++++++++++++++ src/components/IsolatedComponent.tsx | 20 +++++---------- webpack.sharedConfig.js | 4 +++ 4 files changed, 47 insertions(+), 15 deletions(-) create mode 100644 src/components/IsolatedComponent.scss diff --git a/src/components/EmotionShadowRoot.ts b/src/components/EmotionShadowRoot.ts index 9042748165..8d0fad6a7a 100644 --- a/src/components/EmotionShadowRoot.ts +++ b/src/components/EmotionShadowRoot.ts @@ -28,7 +28,8 @@ import { type CSSProperties } from "react"; * the host website. To support react-select and any future potential emotion * components we used the emotion variant of the react-shadow library. */ -const ShadowRoot = EmotionShadowRoot["pixiebrix-widget"]!; +const ShadowRoot = EmotionShadowRoot.div!; +// TODO: Use EmotionShadowRoot["pixiebrix-widget"] to avoid any CSS conflicts. Requires snapshot/test updates export const styleReset: CSSProperties = { all: "initial", diff --git a/src/components/IsolatedComponent.scss b/src/components/IsolatedComponent.scss new file mode 100644 index 0000000000..0a06be0df8 --- /dev/null +++ b/src/components/IsolatedComponent.scss @@ -0,0 +1,35 @@ +/* + * 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 . + */ + +:host { + // Don't inherit any style + all: initial; + + // Set a font baseline style. Bootstrap targets `body` specifically, which isn't available in a shadow DOM + font: 16px / 1.5 sans-serif; + + // Set a good default for our custom `pixiebrix-widget` element + display: block; + + // Avoid black scrollbars on dark websites that set `color-scheme: light dark` + color-scheme: light; +} + +// Don't inherit the selection color +:host::selection { + background: initial; +} diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 75abc31d33..ad7d9564e8 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -15,24 +15,16 @@ * along with this program. If not, see . */ +// This cannot be a CSS module because it must live inside the shadow DOM +// and synchronously set the :host element style. +import cssText from "./IsolatedComponent.scss?loadAsText"; + import React, { Suspense, useEffect } from "react"; import { Stylesheets } from "@/components/Stylesheets"; import EmotionShadowRoot from "@/components/EmotionShadowRoot"; -import { css } from "code-tag"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; -const styleReset = css` - :host { - all: initial; - font: 16px / 1.5 sans-serif; - color-scheme: light; - } - :host::selection { - background: initial; - } -`; - /** * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. * @@ -65,7 +57,7 @@ export const IsolatedComponent: React.VFC<{ }> = ({ webpackChunkName, children, ...props }) => { const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); useEffect(() => { - const observer = new MutationObserver(([mutations]) => { + const observer = new MutationObserver(() => { const link = document.head.querySelector( `link[href="${stylesheetUrl}"]`, ); @@ -83,7 +75,7 @@ export const IsolatedComponent: React.VFC<{ return ( - + {children} diff --git a/webpack.sharedConfig.js b/webpack.sharedConfig.js index 53bd3151f1..dc362ee0e8 100644 --- a/webpack.sharedConfig.js +++ b/webpack.sharedConfig.js @@ -115,6 +115,10 @@ const shared = { test: /\.txt/, type: "asset/source", }, + { + resourceQuery: /loadAsText/, + type: "asset/source", + }, { // CSS-only, must include .css or else it may output .scss files test: /\.s?css$/, From 386b02cab589a30acbd04b73136fb0da3eef4aea Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 18:44:56 +0700 Subject: [PATCH 04/33] Wrap `PropertyTree` --- src/bricks/renderers/PropertyTree.tsx | 19 ++++++++----------- src/bricks/renderers/propertyTable.tsx | 19 +++++++++++++++---- src/components/IsolatedComponent.tsx | 3 +++ 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/bricks/renderers/PropertyTree.tsx b/src/bricks/renderers/PropertyTree.tsx index 97064b961c..7f6a18035e 100644 --- a/src/bricks/renderers/PropertyTree.tsx +++ b/src/bricks/renderers/PropertyTree.tsx @@ -15,25 +15,22 @@ * along with this program. If not, see . */ +import "primereact/resources/themes/saga-blue/theme.css"; +import "primereact/resources/primereact.min.css"; +import "primeicons/primeicons.css"; + import { TreeTable } from "primereact/treetable"; import type TreeNode from "primereact/treenode"; import { Column } from "primereact/column"; -import { Stylesheets } from "@/components/Stylesheets"; import React from "react"; -import theme from "primereact/resources/themes/saga-blue/theme.css?loadAsUrl"; -import primereact from "primereact/resources/primereact.min.css?loadAsUrl"; -import primeicons from "primeicons/primeicons.css?loadAsUrl"; - const PropertyTree: React.FunctionComponent<{ value: TreeNode[] }> = ({ value, }) => ( - - - - - - + + + + ); export default PropertyTree; diff --git a/src/bricks/renderers/propertyTable.tsx b/src/bricks/renderers/propertyTable.tsx index 1512507925..569fb31fcf 100644 --- a/src/bricks/renderers/propertyTable.tsx +++ b/src/bricks/renderers/propertyTable.tsx @@ -21,6 +21,8 @@ import { sortBy, isPlainObject } from "lodash"; import { type BrickArgs, type BrickOptions } from "@/types/runtimeTypes"; import { isValidUrl } from "@/utils/urlUtils"; import { propertiesToSchema } from "@/utils/schemaUtils"; +import { IsolatedComponent } from "@/components/IsolatedComponent"; +import TreeNode from "primereact/treenode"; interface Item { key: string; @@ -115,13 +117,22 @@ export class PropertyTableRenderer extends RendererABC { ); async render({ data }: BrickArgs, { ctxt }: BrickOptions) { - const PropertyTree = await import( - /* webpackChunkName: "widgets" */ - "./PropertyTree" + const LazyPropertyTree = React.lazy( + async () => + import( + /* webpackChunkName: "PropertyTree" */ + "./PropertyTree" + ), + ); + + const PropertyTree: React.FC<{ value: TreeNode[] }> = ({ value }) => ( + + + ); return { - Component: PropertyTree.default, + Component: PropertyTree, props: { value: shapeData(data ?? ctxt), }, diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index ad7d9564e8..17ece27102 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -61,6 +61,9 @@ export const IsolatedComponent: React.VFC<{ const link = document.head.querySelector( `link[href="${stylesheetUrl}"]`, ); + + // TODO: Throw an error on timeout, the developer likely set the wrong webpackChunkName + if (link) { // Disable stylesheet without removing it. Webpack still awaits its loading. link.media = "not all"; From 6a273f9026faa36725707a50ed13b8562bb077ab Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 18:59:20 +0700 Subject: [PATCH 05/33] Wrap `SelectionToolPopover` --- src/components/IsolatedComponent.tsx | 5 +- .../SelectionToolPopover.module.scss | 70 ++------- .../SelectionToolPopover.tsx | 145 +++++++----------- .../pageEditor/elementPicker.test.ts | 7 - .../{elementPicker.ts => elementPicker.tsx} | 40 +++-- 5 files changed, 92 insertions(+), 175 deletions(-) rename src/contentScript/pageEditor/{elementPicker.ts => elementPicker.tsx} (93%) diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 17ece27102..15c828a703 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -52,8 +52,6 @@ export const IsolatedComponent: React.VFC<{ * It must be the result of React.lazy() */ children: JSX.Element; - - className?: string; }> = ({ webpackChunkName, children, ...props }) => { const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); useEffect(() => { @@ -77,7 +75,8 @@ export const IsolatedComponent: React.VFC<{ }); return ( - + // `pb-name` used to visually identify it in the Chrome DevTools + {children} diff --git a/src/components/selectionToolPopover/SelectionToolPopover.module.scss b/src/components/selectionToolPopover/SelectionToolPopover.module.scss index a453de229f..631c893da3 100644 --- a/src/components/selectionToolPopover/SelectionToolPopover.module.scss +++ b/src/components/selectionToolPopover/SelectionToolPopover.module.scss @@ -19,17 +19,7 @@ $background: rgb(252, 252, 252); $border: rgba(0, 0, 0, 0.3); -.popover-wrapper { - // I grabbed this fonts from the page editor stylesheet. - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, - "Helvetica Neue", Arial, "Noto Sans", "Liberation Sans", sans-serif, - "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji"; - font-size: 14px; - font-style: normal; - font-weight: 400; - line-height: 1.5; - color: #212529; - letter-spacing: normal; +.root { position: fixed; bottom: 30px; left: 30px; @@ -38,53 +28,21 @@ $border: rgba(0, 0, 0, 0.3); background: $background; cursor: pointer; z-index: 10000001; +} - .popover-wrapper-header { - display: flex; - align-items: center; - cursor: move; - padding: 12px; - background: $popover-header; - - svg { - width: 24px; - margin-right: 8px; - } - } - - .popover-wrapper-body { - padding: 12px; - } - - // We're overriding bootstrap components style here to prevent style breaks in react-shadow-dom - .card-header:first-child { - border-radius: calc(0.25em - 1px) calc(0.25em - 1px) 0 0; - } - .card-header { - padding: 0.75em 1.25em; - } - .card-body { - padding: 1.25em; - } +.header { + display: flex; + align-items: center; + cursor: move; + padding: 12px; + background: $popover-header; - .btn { - font-size: 1em; - } - .switch.btn { - min-width: 3.7em; - min-height: calc(1.5em + 0.75em + 2px); - } - .switch-on.btn { - padding-right: 1.5em; - } - .switch-off.btn { - padding-left: 1.5em; + svg { + width: 24px; + margin-right: 8px; } +} - .btn-sm { - padding: 0.5em 0.75em; - font-size: 1em; - line-height: 1.5; - border-radius: 0.2em; - } +.body { + padding: 12px; } diff --git a/src/components/selectionToolPopover/SelectionToolPopover.tsx b/src/components/selectionToolPopover/SelectionToolPopover.tsx index efcec4f43c..692c7f293c 100644 --- a/src/components/selectionToolPopover/SelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/SelectionToolPopover.tsx @@ -14,19 +14,15 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ +import "@/vendors/bootstrapWithoutRem.css"; +import "bootstrap-switch-button-react/src/style.css"; +import styles from "./SelectionToolPopover.module.scss"; import React, { type ChangeEvent, useEffect, useState } from "react"; -import ReactDOM from "react-dom"; -import bootstrap from "@/vendors/bootstrapWithoutRem.css?loadAsUrl"; import Draggable from "react-draggable"; -import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import SwitchButtonWidget, { type CheckBoxLike, } from "@/components/form/widgets/switchButton/SwitchButtonWidget"; -import switchStyle from "@/components/form/widgets/switchButton/SwitchButtonWidget.module.scss?loadAsUrl"; -import switchButtonStyle from "bootstrap-switch-button-react/src/style.css?loadAsUrl"; -import custom from "./SelectionToolPopover.module.scss?loadAsUrl"; -import { Stylesheets } from "@/components/Stylesheets"; import { Button, FormLabel } from "react-bootstrap"; import pluralize from "@/utils/pluralize"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -66,100 +62,63 @@ const SelectionToolPopover: React.FC<{ }, [setSelectionHandler]); return ( - // To support react-select and any future potential emotion components we used the - // emotion variant of the react-shadow library. + +
+
+ + {`Selection Tool: ${matchingCount} ${pluralize( + matchingCount, + "matching element", + "matching elements", + )}`} +
+
+ ) => { + setMultiEnabled(target.value); + if (!target.value) { + setSimilarEnabled(false); + } - - - -
-
- - {`Selection Tool: ${matchingCount} ${pluralize( - matchingCount, - "matching element", - "matching elements", - )}`} -
-
+ onChangeMultiSelection(target.value); + }} + /> + + Select Multiple + + + {multiEnabled && ( + <> ) => { - setMultiEnabled(target.value); - if (!target.value) { - setSimilarEnabled(false); - } - - onChangeMultiSelection(target.value); + setSimilarEnabled(target.value); + onChangeSimilarSelection(target.value); }} /> - Select Multiple + Select Similar + + )} - {multiEnabled && ( - <> - ) => { - setSimilarEnabled(target.value); - onChangeSimilarSelection(target.value); - }} - /> - - Select Similar - - - )} - - - -
-
- - - - ); -}; - -export const showSelectionToolPopover = ({ - rootElement, - isMulti, - handleCancel, - handleDone, - handleMultiChange, - handleSimilarChange, - setSelectionHandler, -}: { - rootElement: HTMLElement; - isMulti: boolean; - handleCancel: () => void; - handleDone: () => void; - handleMultiChange: (value: boolean) => void; - handleSimilarChange: (value: boolean) => void; - setSelectionHandler: SetSelectionHandlerType; -}) => { - ReactDOM.render( - , - rootElement, + + +
+
+ ); }; diff --git a/src/contentScript/pageEditor/elementPicker.test.ts b/src/contentScript/pageEditor/elementPicker.test.ts index fb68e57ccf..0b292070fb 100644 --- a/src/contentScript/pageEditor/elementPicker.test.ts +++ b/src/contentScript/pageEditor/elementPicker.test.ts @@ -20,21 +20,14 @@ import { userSelectElement, } from "@/contentScript/pageEditor/elementPicker"; import { BusinessError } from "@/errors/businessErrors"; -import { showSelectionToolPopover } from "@/components/selectionToolPopover/SelectionToolPopover"; // Mock because the React vs. JSDOM event handling and dom manipulation isn't playing nicely together jest.mock("@/components/selectionToolPopover/SelectionToolPopover"); -const showSelectionToolPopoverMock = jest.mocked(showSelectionToolPopover); - beforeAll(() => { Element.prototype.scrollTo = jest.fn(); }); -beforeEach(() => { - showSelectionToolPopoverMock.mockClear(); -}); - describe("userSelectElement", () => { test("can select single element", async () => { document.body.innerHTML = "
hello
"; diff --git a/src/contentScript/pageEditor/elementPicker.ts b/src/contentScript/pageEditor/elementPicker.tsx similarity index 93% rename from src/contentScript/pageEditor/elementPicker.ts rename to src/contentScript/pageEditor/elementPicker.tsx index ec9c3a5aed..207c11f959 100644 --- a/src/contentScript/pageEditor/elementPicker.ts +++ b/src/contentScript/pageEditor/elementPicker.tsx @@ -17,7 +17,7 @@ */ import Overlay from "@/vendors/Overlay"; -import ReactDOM from "react-dom"; +import ReactDOM, { render } from "react-dom"; import { expandedCssSelector, findContainer, @@ -26,10 +26,7 @@ import { import { compact, difference, uniq } from "lodash"; import * as pageScript from "@/pageScript/messenger/api"; import { type SelectMode } from "@/contentScript/pageEditor/types"; -import { - type SelectionHandlerType, - showSelectionToolPopover, -} from "@/components/selectionToolPopover/SelectionToolPopover"; +import type { SelectionHandlerType } from "@/components/selectionToolPopover/SelectionToolPopover"; import { BusinessError, CancelError, @@ -40,6 +37,8 @@ import { $safeFind, findSingleElement } from "@/utils/domUtils"; import inferSingleElementSelector from "@/utils/inference/inferSingleElementSelector"; import { type ElementInfo } from "@/utils/inference/selectorTypes"; import { onContextInvalidated } from "webext-events"; +import React from "react"; +import { IsolatedComponent } from "@/components/IsolatedComponent"; /** * Primary overlay that moved with the user's mouse/selection. @@ -382,17 +381,26 @@ export async function userSelectElement({ // Hide the FAB so it doesn't conflict with the selection tool. Is a NOP if the FAB is not on the page $(`#${FLOATING_ACTION_BUTTON_CONTAINER_ID}`).hide(); - showSelectionToolPopover({ - rootElement: multiSelectionToolElement, - isMulti, - handleCancel: cancel, - handleDone() { - handleDone(); - }, - handleMultiChange: handleMultiSelectionChange, - handleSimilarChange: handleSimilarSelectionChange, - setSelectionHandler, - }); + const SelectionToolPopover = React.lazy( + async () => + import( + /* webpackChunkName: "SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" + ), + ); + + render( + + + , + multiSelectionToolElement, + ); } function removeMultiSelectionTool() { From 360f843ac313acdc145f46ca78f12dd83b167696 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 19:22:14 +0700 Subject: [PATCH 06/33] Fix tests --- .github/workflows/ci.yml | 1 + end-to-end-tests/utils.ts | 2 +- jest.config.js | 20 +++-- src/__snapshots__/Storyshots.test.js.snap | 83 ++++++++++++++++--- src/bricks/renderers/propertyTable.tsx | 2 +- .../IntegrationConfigEditorModal.test.tsx | 6 +- .../SelectionToolPopover.tsx | 4 +- .../showSelectionToolPopover.tsx | 42 ++++++++++ .../pageEditor/elementPicker.test.ts | 17 ++-- .../pageEditor/elementPicker.tsx | 34 +++----- .../pages/brickEditor/useSubmitBrick.test.tsx | 5 +- src/testUtils/testEnv.js | 1 + 12 files changed, 162 insertions(+), 55 deletions(-) create mode 100644 src/components/selectionToolPopover/showSelectionToolPopover.tsx diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1de28e1eca..0ab1d587fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -197,6 +197,7 @@ jobs: timeout-minutes: 60 runs-on: ubuntu-latest env: + SHADOW_DOM: open SERVICE_URL: https://app.pixiebrix.com MV: ${{ matrix.MV }} CHROME_MANIFEST_KEY: ${{ matrix.CHROME_MANIFEST_KEY }} diff --git a/end-to-end-tests/utils.ts b/end-to-end-tests/utils.ts index 30cab5ff37..d3f8dfda5b 100644 --- a/end-to-end-tests/utils.ts +++ b/end-to-end-tests/utils.ts @@ -16,7 +16,7 @@ */ import type AxeBuilder from "@axe-core/playwright"; -import { type Locator, expect, type Page, Frame } from "@playwright/test"; +import { type Locator, expect, type Page, type Frame } from "@playwright/test"; import { MV } from "./env"; type AxeResults = Awaited>; diff --git a/jest.config.js b/jest.config.js index 671a2ce271..d892135074 100644 --- a/jest.config.js +++ b/jest.config.js @@ -60,12 +60,12 @@ const config = { modulePathIgnorePatterns: ["/headers.json", "/dist/"], testPathIgnorePatterns: ["/end-to-end-tests"], transform: { - "^.+\\.[jt]sx?$": "@swc/jest", - "^.+\\.mjs$": "@swc/jest", - "^.+\\.ya?ml$": "yaml-jest-transform", - "^.+\\.ya?ml\\?loadAsText$": - "/src/testUtils/rawJestTransformer.mjs", - "^.+\\.txt$": "/src/testUtils/rawJestTransformer.mjs", + "\\.[jt]sx?$": "@swc/jest", + "\\.mjs$": "@swc/jest", + "\\.ya?ml$": "yaml-jest-transform", + "\\.txt$": "/src/testUtils/rawJestTransformer.mjs", + // Note: `?param` URLs aren't supported here: https://github.com/jestjs/jest/pull/6282 + // You can only use a mock via `moduleNameMapper` for these. }, transformIgnorePatterns: [`node_modules/(?!${esmPackages.join("|")})`], setupFiles: [ @@ -91,8 +91,12 @@ const config = { ], moduleNameMapper: { "\\.s?css$": "identity-obj-proxy", - "\\.(gif|svg|png)$|\\?loadAsUrl$|\\?loadAsComponent$": - "/src/__mocks__/stringMock.js", + "\\.(gif|svg|png)$": "/src/__mocks__/stringMock.js", + + "\\?loadAsUrl$": "/src/__mocks__/stringMock.js", + "\\?loadAsText$": "/src/__mocks__/stringMock.js", + "\\?loadAsComponent$": "/src/__mocks__/stringMock.js", + "^@contrib/(.*?)(\\?loadAsText)?$": "/contrib/$1", "^@schemas/(.*)": "/schemas/$1", diff --git a/src/__snapshots__/Storyshots.test.js.snap b/src/__snapshots__/Storyshots.test.js.snap index 6fed1df49d..ba12b76355 100644 --- a/src/__snapshots__/Storyshots.test.js.snap +++ b/src/__snapshots__/Storyshots.test.js.snap @@ -3493,22 +3493,85 @@ exports[`Storyshots Editor/LogToolbar Default 1`] = ` exports[`Storyshots Enhancements/SelectionMenu Emoji Buttons 1`] = `
+ + +
`; exports[`Storyshots Enhancements/SelectionMenu Mixed Buttons 1`] = `
+ + +
`; exports[`Storyshots Enhancements/ShortcutSnippetMenu Demo 1`] = ` diff --git a/src/bricks/renderers/propertyTable.tsx b/src/bricks/renderers/propertyTable.tsx index 569fb31fcf..7ae9d94b3f 100644 --- a/src/bricks/renderers/propertyTable.tsx +++ b/src/bricks/renderers/propertyTable.tsx @@ -22,7 +22,7 @@ import { type BrickArgs, type BrickOptions } from "@/types/runtimeTypes"; import { isValidUrl } from "@/utils/urlUtils"; import { propertiesToSchema } from "@/utils/schemaUtils"; import { IsolatedComponent } from "@/components/IsolatedComponent"; -import TreeNode from "primereact/treenode"; +import type TreeNode from "primereact/treenode"; interface Item { key: string; diff --git a/src/components/integrations/IntegrationConfigEditorModal.test.tsx b/src/components/integrations/IntegrationConfigEditorModal.test.tsx index be8d6ee875..7159fc053c 100644 --- a/src/components/integrations/IntegrationConfigEditorModal.test.tsx +++ b/src/components/integrations/IntegrationConfigEditorModal.test.tsx @@ -21,9 +21,9 @@ import IntegrationConfigEditorModal from "@/components/integrations/IntegrationC import { render, screen } from "@/extensionConsole/testHelpers"; import { waitForEffect } from "@/testUtils/testHelpers"; -// FIXME: this is coming through as a module with default being a JSON object. (yaml-jest-transform is being applied) -import pipedriveYaml from "@contrib/integrations/pipedrive.yaml?loadAsText"; -import automationAnywhereYaml from "@contrib/integrations/automation-anywhere.yaml?loadAsText"; +// FIXME: Use ?loadAsText when supported by Jest https://github.com/jestjs/jest/pull/6282 +import pipedriveYaml from "@contrib/integrations/pipedrive.yaml"; +import automationAnywhereYaml from "@contrib/integrations/automation-anywhere.yaml"; import registerDefaultWidgets from "@/components/fields/schemaFields/widgets/registerDefaultWidgets"; import userEvent from "@testing-library/user-event"; import { type IntegrationConfig } from "@/integrations/integrationTypes"; diff --git a/src/components/selectionToolPopover/SelectionToolPopover.tsx b/src/components/selectionToolPopover/SelectionToolPopover.tsx index 692c7f293c..bec6b74db5 100644 --- a/src/components/selectionToolPopover/SelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/SelectionToolPopover.tsx @@ -29,7 +29,9 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { faGripHorizontal } from "@fortawesome/free-solid-svg-icons"; export type SelectionHandlerType = (count: number) => void; -type SetSelectionHandlerType = (handler: SelectionHandlerType | null) => void; +export type SetSelectionHandlerType = ( + handler: SelectionHandlerType | null, +) => void; const SelectionToolPopover: React.FC<{ isMulti: boolean; diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx new file mode 100644 index 0000000000..20986b4388 --- /dev/null +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -0,0 +1,42 @@ +/* + * 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 React from "react"; +import { render } from "react-dom"; +import { IsolatedComponent } from "@/components/IsolatedComponent"; +import type Component from "@/components/selectionToolPopover/SelectionToolPopover"; + +export default function showSelectionToolPopover({ + rootElement, + ...props +}: { + rootElement: HTMLElement; +} & React.ComponentProps) { + const SelectionToolPopover = React.lazy( + async () => + import( + /* webpackChunkName: "SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" + ), + ); + + render( + + + , + rootElement, + ); +} diff --git a/src/contentScript/pageEditor/elementPicker.test.ts b/src/contentScript/pageEditor/elementPicker.test.ts index 0b292070fb..6e401026ae 100644 --- a/src/contentScript/pageEditor/elementPicker.test.ts +++ b/src/contentScript/pageEditor/elementPicker.test.ts @@ -20,12 +20,17 @@ import { userSelectElement, } from "@/contentScript/pageEditor/elementPicker"; import { BusinessError } from "@/errors/businessErrors"; +import showSelectionToolPopover from "@/components/selectionToolPopover/showSelectionToolPopover"; // Mock because the React vs. JSDOM event handling and dom manipulation isn't playing nicely together -jest.mock("@/components/selectionToolPopover/SelectionToolPopover"); +jest.mock("@/components/selectionToolPopover/showSelectionToolPopover"); -beforeAll(() => { - Element.prototype.scrollTo = jest.fn(); +const showSelectionToolPopoverMock = jest.mocked(showSelectionToolPopover); + +Element.prototype.scrollTo = jest.fn(); + +beforeEach(() => { + showSelectionToolPopoverMock.mockClear(); }); describe("userSelectElement", () => { @@ -168,7 +173,7 @@ describe("selectElement", () => { expect(selectionHandlerMock).toHaveBeenCalledTimes(2); - args.handleDone(); + args.onDone(); await expect(selectPromise).resolves.toEqual({ parent: null, @@ -195,7 +200,7 @@ describe("selectElement", () => { args.setSelectionHandler(selectionHandlerMock); - args.handleSimilarChange(true); + args.onChangeSimilarSelection(true); expect(selectionHandlerMock).toHaveBeenCalledTimes(1); // React testing userEvent library doesn't seem to work here @@ -212,7 +217,7 @@ describe("selectElement", () => { expect(selectionHandlerMock).toHaveBeenCalledTimes(3); - args.handleDone(); + args.onDone(); await expect(selectPromise).resolves.toEqual({ parent: null, diff --git a/src/contentScript/pageEditor/elementPicker.tsx b/src/contentScript/pageEditor/elementPicker.tsx index 207c11f959..fd8d298df3 100644 --- a/src/contentScript/pageEditor/elementPicker.tsx +++ b/src/contentScript/pageEditor/elementPicker.tsx @@ -17,7 +17,7 @@ */ import Overlay from "@/vendors/Overlay"; -import ReactDOM, { render } from "react-dom"; +import ReactDOM from "react-dom"; import { expandedCssSelector, findContainer, @@ -37,8 +37,7 @@ import { $safeFind, findSingleElement } from "@/utils/domUtils"; import inferSingleElementSelector from "@/utils/inference/inferSingleElementSelector"; import { type ElementInfo } from "@/utils/inference/selectorTypes"; import { onContextInvalidated } from "webext-events"; -import React from "react"; -import { IsolatedComponent } from "@/components/IsolatedComponent"; +import showSelectionToolPopover from "@/components/selectionToolPopover/showSelectionToolPopover"; /** * Primary overlay that moved with the user's mouse/selection. @@ -381,26 +380,15 @@ export async function userSelectElement({ // Hide the FAB so it doesn't conflict with the selection tool. Is a NOP if the FAB is not on the page $(`#${FLOATING_ACTION_BUTTON_CONTAINER_ID}`).hide(); - const SelectionToolPopover = React.lazy( - async () => - import( - /* webpackChunkName: "SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" - ), - ); - - render( - - - , - multiSelectionToolElement, - ); + showSelectionToolPopover({ + rootElement: multiSelectionToolElement, + isMulti, + onCancel: cancel, + onDone: handleDone, + onChangeMultiSelection: handleMultiSelectionChange, + onChangeSimilarSelection: handleSimilarSelectionChange, + setSelectionHandler, + }); } function removeMultiSelectionTool() { diff --git a/src/extensionConsole/pages/brickEditor/useSubmitBrick.test.tsx b/src/extensionConsole/pages/brickEditor/useSubmitBrick.test.tsx index 935b135ad7..79f380b3a8 100644 --- a/src/extensionConsole/pages/brickEditor/useSubmitBrick.test.tsx +++ b/src/extensionConsole/pages/brickEditor/useSubmitBrick.test.tsx @@ -27,8 +27,9 @@ import { type SettingsState } from "@/store/settings/settingsTypes"; import { configureStore } from "@reduxjs/toolkit"; import { authSlice } from "@/auth/authSlice"; import settingsSlice from "@/store/settings/settingsSlice"; -// FIXME: this is coming through as a module with default being a JSON object. (yaml-jest-transform is being applied) -import pipedriveYaml from "@contrib/integrations/pipedrive.yaml?loadAsText"; + +// FIXME: Use ?loadAsText when supported by Jest https://github.com/jestjs/jest/pull/6282 +import pipedriveYaml from "@contrib/integrations/pipedrive.yaml"; import { appApi } from "@/data/service/api"; import { brickToYaml } from "@/utils/objToYaml"; import testMiddleware, { diff --git a/src/testUtils/testEnv.js b/src/testUtils/testEnv.js index e9cbcb7f20..d28a812521 100644 --- a/src/testUtils/testEnv.js +++ b/src/testUtils/testEnv.js @@ -21,6 +21,7 @@ import { TextEncoder, TextDecoder } from "node:util"; // eslint-disable-next-line import/no-unassigned-import -- It's a polyfill import "urlpattern-polyfill"; +process.env.SHADOW_ROOT = "open"; process.env.SERVICE_URL = "https://app.pixiebrix.com"; process.env.MARKETPLACE_URL = "https://www.pixiebrix.com/marketplace/"; From e3018e888eb1eaad77563ff2e12539b623197aec Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 3 Apr 2024 19:27:46 +0700 Subject: [PATCH 07/33] Fix unrelated mutation https://togithub.com/sindresorhus/eslint-plugin-unicorn/issues/1514 https://togithub.com/pixiebrix/pixiebrix-extension/pull/4486 --- src/components/jsonTree/treeHooks.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/jsonTree/treeHooks.tsx b/src/components/jsonTree/treeHooks.tsx index 719f5f111e..5c9ed9ac57 100644 --- a/src/components/jsonTree/treeHooks.tsx +++ b/src/components/jsonTree/treeHooks.tsx @@ -42,7 +42,7 @@ export function useLabelRenderer() { href="#" onClick={async (event) => { await writeToClipboard({ - text: getPathFromArray(keyPath.reverse()), + text: getPathFromArray([...keyPath].reverse()), }); event.preventDefault(); event.stopPropagation(); From 4760ee073d5a4e101197ed6fa10f609ff0379b88 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Thu, 4 Apr 2024 00:33:56 +0700 Subject: [PATCH 08/33] Update docs --- .env.example | 7 ++++-- end-to-end-tests/utils.ts | 2 +- src/components/IsolatedComponent.tsx | 23 ++++++++++--------- .../showSelectionToolPopover.tsx | 5 ++++ .../{elementPicker.tsx => elementPicker.ts} | 0 src/tsconfig.strictNullChecks.json | 2 ++ 6 files changed, 25 insertions(+), 14 deletions(-) rename src/contentScript/pageEditor/{elementPicker.tsx => elementPicker.ts} (100%) diff --git a/.env.example b/.env.example index bfbf6545fa..d4a5202642 100644 --- a/.env.example +++ b/.env.example @@ -4,8 +4,11 @@ CHROME_MANIFEST_KEY= CHROME_EXTENSION_ID=mpjjildhmpddojocokjkgmlkkkfjnepo -# Chrome extension manifest version. Default is MV=2 -# MV=3 +# Chrome extension manifest version. Default is MV=3 +# MV=2 + +# Shadow DOM mode for all components. Default is SHADOW_DOM=closed in regular webpack builds, open elsewhere +# SHADOW_DOM=open # This makes all optional permissions required in the manifest.json to avoid permission popups. Only required for Playwright tests. # REQUIRE_OPTIONAL_PERMISSIONS_IN_MANIFEST=1 diff --git a/end-to-end-tests/utils.ts b/end-to-end-tests/utils.ts index d3f8dfda5b..30cab5ff37 100644 --- a/end-to-end-tests/utils.ts +++ b/end-to-end-tests/utils.ts @@ -16,7 +16,7 @@ */ import type AxeBuilder from "@axe-core/playwright"; -import { type Locator, expect, type Page, type Frame } from "@playwright/test"; +import { type Locator, expect, type Page, Frame } from "@playwright/test"; import { MV } from "./env"; type AxeResults = Awaited>; diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 15c828a703..111497973c 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -29,18 +29,17 @@ const MODE = process.env.SHADOW_DOM as "open" | "closed"; * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. * * @example - * const Component = React.lazy(() => import( - * /* webpackChunkName: Component / - * "./Component" + * const Moon = React.lazy(() => import( + * /* webpackChunkName: Moon / + * "./Moon" * )); * * render( - * , - * document.querySelector('#container') - * ) + * + * + * , + * document.querySelector('#container'), + * ); */ export const IsolatedComponent: React.VFC<{ /** @@ -54,14 +53,16 @@ export const IsolatedComponent: React.VFC<{ children: JSX.Element; }> = ({ webpackChunkName, children, ...props }) => { const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); + + // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. + // This stylesheet is injected only once per document, so this observer might not + // be triggered for *every* instance of IsolatedComponent. useEffect(() => { const observer = new MutationObserver(() => { const link = document.head.querySelector( `link[href="${stylesheetUrl}"]`, ); - // TODO: Throw an error on timeout, the developer likely set the wrong webpackChunkName - if (link) { // Disable stylesheet without removing it. Webpack still awaits its loading. link.media = "not all"; diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx index 20986b4388..8424d87db1 100644 --- a/src/components/selectionToolPopover/showSelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -15,6 +15,11 @@ * along with this program. If not, see . */ +/** + * @file This file exists to facilitate testing/mocking. The function cannot live + * in SelectionToolPopover.tsx because it must import it asynchronously. + */ + import React from "react"; import { render } from "react-dom"; import { IsolatedComponent } from "@/components/IsolatedComponent"; diff --git a/src/contentScript/pageEditor/elementPicker.tsx b/src/contentScript/pageEditor/elementPicker.ts similarity index 100% rename from src/contentScript/pageEditor/elementPicker.tsx rename to src/contentScript/pageEditor/elementPicker.ts diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 78cbb25fa1..414616e706 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -196,6 +196,7 @@ "./components/ErrorBoundary.tsx", "./components/InvalidatedContextGate.test.tsx", "./components/InvalidatedContextGate.tsx", + "./components/IsolatedComponent.tsx", "./components/LayoutWidget.tsx", "./components/LinkButton.tsx", "./components/Loader.tsx", @@ -301,6 +302,7 @@ "./components/quickBar/useActionGenerators.ts", "./components/quickBar/useActions.ts", "./components/quickBar/utils.ts", + "./components/selectionToolPopover/showSelectionToolPopover.tsx", "./components/selectionToolPopover/SelectionToolPopover.tsx", "./components/walkthroughModal/showWalkthroughModal.ts", "./contentScript/activationConstants.ts", From a2c75b96068ccbc954c4e50cf8ef89ffe25d7fe2 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Thu, 4 Apr 2024 01:24:12 +0700 Subject: [PATCH 09/33] Cleanup --- src/components/IsolatedComponent.tsx | 33 ++++++++++++++++++---------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 111497973c..7060833e31 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -15,8 +15,8 @@ * along with this program. If not, see . */ -// This cannot be a CSS module because it must live inside the shadow DOM -// and synchronously set the :host element style. +// This cannot be a CSS module or URL because it must live inside the +// shadow DOM and synchronously set the :host element style. import cssText from "./IsolatedComponent.scss?loadAsText"; import React, { Suspense, useEffect } from "react"; @@ -25,6 +25,20 @@ import EmotionShadowRoot from "@/components/EmotionShadowRoot"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; +function deactivateGlobalStyle(href: string): boolean { + const link = document.head.querySelector( + `link[href="${href}"]`, + ); + + if (link) { + // Disable stylesheet without removing it. Webpack still awaits its loading. + link.media = "not all"; + return true; + } + + return false; +} + /** * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. * @@ -55,17 +69,14 @@ export const IsolatedComponent: React.VFC<{ const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. - // This stylesheet is injected only once per document, so this observer might not - // be triggered for *every* instance of IsolatedComponent. useEffect(() => { - const observer = new MutationObserver(() => { - const link = document.head.querySelector( - `link[href="${stylesheetUrl}"]`, - ); + if (deactivateGlobalStyle(stylesheetUrl)) { + // This stylesheet is injected only once per document, don't await further injections. + return; + } - if (link) { - // Disable stylesheet without removing it. Webpack still awaits its loading. - link.media = "not all"; + const observer = new MutationObserver(() => { + if (deactivateGlobalStyle(stylesheetUrl)) { observer.disconnect(); } }); From 97603930543518593e5526f4dafa5ab3df21b807 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Thu, 4 Apr 2024 22:07:59 +0700 Subject: [PATCH 10/33] Some components need no style; test Stylesheets component --- src/components/InvalidatedContextGate.tsx | 5 +- src/components/IsolatedComponent.tsx | 22 ++++- src/components/Stylesheets.test.tsx | 103 ++++++++++++++++++++++ src/components/Stylesheets.tsx | 6 +- 4 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 src/components/Stylesheets.test.tsx diff --git a/src/components/InvalidatedContextGate.tsx b/src/components/InvalidatedContextGate.tsx index 4903711e73..9b4353f75e 100644 --- a/src/components/InvalidatedContextGate.tsx +++ b/src/components/InvalidatedContextGate.tsx @@ -28,7 +28,10 @@ const InvalidatedContextGate: React.FunctionComponent<{ // Only auto-reload if the document is in the background const isDocumentVisible = useDocumentVisibility(); if (wasContextInvalidated && autoReload && !isDocumentVisible) { - location.reload(); + setTimeout(() => { + // If you reload too soon, Chrome might not be ready to serve the page yet + location.reload(); + }, 500); } return wasContextInvalidated ? ( diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 7060833e31..15ea7d5a9b 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -61,15 +61,29 @@ export const IsolatedComponent: React.VFC<{ */ webpackChunkName: string; + /** + * If true, the component will not attempt to load the stylesheet. + * + * @example + */ + noStyle?: boolean; + /** * It must be the result of React.lazy() */ children: JSX.Element; -}> = ({ webpackChunkName, children, ...props }) => { - const stylesheetUrl = chrome.runtime.getURL(`css/${webpackChunkName}.css`); +}> = ({ webpackChunkName, children, noStyle, ...props }) => { + const stylesheetUrl = noStyle + ? null + : chrome.runtime.getURL(`css/${webpackChunkName}.css`); // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. useEffect(() => { + if (!stylesheetUrl) { + // This component doesn't generate any stylesheets + return; + } + if (deactivateGlobalStyle(stylesheetUrl)) { // This stylesheet is injected only once per document, don't await further injections. return; @@ -87,10 +101,10 @@ export const IsolatedComponent: React.VFC<{ }); return ( - // `pb-name` used to visually identify it in the Chrome DevTools + // `pb-name` is used to visually identify it in the dev tools - + {children} diff --git a/src/components/Stylesheets.test.tsx b/src/components/Stylesheets.test.tsx new file mode 100644 index 0000000000..21478a64b5 --- /dev/null +++ b/src/components/Stylesheets.test.tsx @@ -0,0 +1,103 @@ +/* + * 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 React from "react"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { Stylesheets } from "./Stylesheets"; + +it("renders the children immediately when no stylesheets are provided", () => { + render( + +
hello
+ , + ); + + expect(screen.getByText("hello")).toBeInTheDocument(); +}); + +it("renders the children after the stylesheet is loaded", () => { + const { container } = render( + + + , + ); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element + const stylesheet = container.querySelector("link"); + expect(stylesheet).toHaveAttribute("href", "https://example.com/style.css"); + + // It should be in the document but not visible + expect(screen.getByText("Buy flowers")).toBeInTheDocument(); + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + + fireEvent.load(stylesheet); + + // Now it should be visible + expect(screen.getByRole("button")).toBeInTheDocument(); +}); + +it("renders the children after the stylesheet is loaded when multiple stylesheets are provided", () => { + const { container } = render( + + + , + ); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element + const stylesheets = container.querySelectorAll("link"); + expect(stylesheets).toHaveLength(2); + expect(stylesheets[0]).toHaveAttribute( + "href", + "https://example.com/style1.css", + ); + expect(stylesheets[1]).toHaveAttribute( + "href", + "https://example.com/style2.css", + ); + + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + + fireEvent.load(stylesheets[0]); + fireEvent.load(stylesheets[1]); + + // Now it should be visible + expect(screen.getByRole("button")).toBeInTheDocument(); +}); + +it("renders the children immediately even when the loading of the stylesheet fails", () => { + const { container } = render( + + + , + ); + + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element + const stylesheet = container.querySelector("link"); + expect(stylesheet).toHaveAttribute("href", "https://example.com/style.css"); + + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + + fireEvent.error(stylesheet); + + // It should show anyway + expect(screen.getByRole("button")).toBeInTheDocument(); +}); diff --git a/src/components/Stylesheets.tsx b/src/components/Stylesheets.tsx index 2cc28df60c..2948aa72ca 100644 --- a/src/components/Stylesheets.tsx +++ b/src/components/Stylesheets.tsx @@ -33,10 +33,14 @@ export const Stylesheets: React.FC<{ */ mountOnLoad?: boolean; }> = ({ href, children, mountOnLoad = false }) => { - const urls = uniq(castArray(href)); const [resolved, setResolved] = useState([]); + if (href.length === 0) { + // Shortcut if no stylesheets are needed + return <>{children}; + } // `every` returns true for empty arrays + const urls = uniq(castArray(href)); const allResolved = urls.every((url) => resolved.includes(url)); return ( From df1ec00ce63dfd0f355a90c207e659a7e263b542 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 15:18:40 +0700 Subject: [PATCH 11/33] Ok but no fonts? --- scripts/DiscardFilePlugin.mjs | 15 +++- src/bricks/renderers/propertyTable.tsx | 9 +- src/components/IsolatedComponent.tsx | 87 +++++++++++-------- src/components/isolatedComponentList.d.ts | 3 + src/components/isolatedComponentList.mjs | 37 ++++++++ .../showSelectionToolPopover.tsx | 9 +- webpack.config.mjs | 37 ++++---- webpack.sharedConfig.js | 2 +- 8 files changed, 135 insertions(+), 64 deletions(-) create mode 100644 src/components/isolatedComponentList.d.ts create mode 100644 src/components/isolatedComponentList.mjs diff --git a/scripts/DiscardFilePlugin.mjs b/scripts/DiscardFilePlugin.mjs index f7cd9f3058..a7fa5d1619 100644 --- a/scripts/DiscardFilePlugin.mjs +++ b/scripts/DiscardFilePlugin.mjs @@ -1,4 +1,7 @@ import webpack from "webpack"; +// eslint-disable-next-line no-restricted-imports -- TODO: Rule should not apply here +import isolatedComponentList from "../src/components/isolatedComponentList.mjs"; + // https://github.com/pixiebrix/pixiebrix-extension/pull/7363#discussion_r1458224740 export default class DiscardFilePlugin { apply(compiler) { @@ -9,11 +12,19 @@ export default class DiscardFilePlugin { stage: webpack.Compilation.PROCESS_ASSETS_STAGE_OPTIMIZE_SIZE, }, async (assets) => { + // These files are not used, they're only webpack entry points in order to generate + // a full CSS files that can be injected in shadow DOM. See this for more context: + // https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 + for (const componentPath of isolatedComponentList) { + delete assets[`${componentPath.split("/").pop()}.js`]; + // If `delete assets[]` causes issues in the future, try replacing the content instead: + // assets["DocumentView.js"] = new webpack.sources.RawSource('"Dropped"'); + } + + // TODO: Remove these 3 from here and use delete assets["DocumentView.js"]; delete assets["EphemeralFormContent.js"]; delete assets["CustomFormComponent.js"]; - // If this causes issues in the future, try replacing the content instead: - // assets["DocumentView.js"] = new webpack.sources.RawSource('"Dropped"'); }, ); }); diff --git a/src/bricks/renderers/propertyTable.tsx b/src/bricks/renderers/propertyTable.tsx index 7ae9d94b3f..4f714f6dbe 100644 --- a/src/bricks/renderers/propertyTable.tsx +++ b/src/bricks/renderers/propertyTable.tsx @@ -21,7 +21,10 @@ import { sortBy, isPlainObject } from "lodash"; import { type BrickArgs, type BrickOptions } from "@/types/runtimeTypes"; import { isValidUrl } from "@/utils/urlUtils"; import { propertiesToSchema } from "@/utils/schemaUtils"; -import { IsolatedComponent } from "@/components/IsolatedComponent"; +import { + IsolatedComponent, + reactLazyIsolatedComponent, +} from "@/components/IsolatedComponent"; import type TreeNode from "primereact/treenode"; interface Item { @@ -117,10 +120,10 @@ export class PropertyTableRenderer extends RendererABC { ); async render({ data }: BrickArgs, { ctxt }: BrickOptions) { - const LazyPropertyTree = React.lazy( + const LazyPropertyTree = reactLazyIsolatedComponent( async () => import( - /* webpackChunkName: "PropertyTree" */ + /* webpackChunkName: "isolated/PropertyTree" */ "./PropertyTree" ), ); diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 15ea7d5a9b..61fd0ab807 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -19,24 +19,58 @@ // shadow DOM and synchronously set the :host element style. import cssText from "./IsolatedComponent.scss?loadAsText"; -import React, { Suspense, useEffect } from "react"; +import React, { Suspense } from "react"; import { Stylesheets } from "@/components/Stylesheets"; import EmotionShadowRoot from "@/components/EmotionShadowRoot"; +import isolatedComponentList from "./isolatedComponentList"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; -function deactivateGlobalStyle(href: string): boolean { - const link = document.head.querySelector( - `link[href="${href}"]`, - ); +type LazyComponentImportFunction = () => Promise<{ + default: React.ComponentType; +}>; + +// Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. +async function discardStylesheetsWhilePending( + importFunction: LazyComponentImportFunction, +) { + const baseUrl = chrome.runtime.getURL(""); + + const observer = new MutationObserver((mutations) => { + // Use for of + for (const mutation of mutations) { + for (const node of mutation.addedNodes) { + if (node instanceof HTMLLinkElement && node.href.startsWith(baseUrl)) { + // Disable stylesheet without removing it. Webpack still awaits its loading. + node.media = "not all"; + } + } + } + }); + + observer.observe(document.head, { + childList: true, + }); - if (link) { - // Disable stylesheet without removing it. Webpack still awaits its loading. - link.media = "not all"; - return true; + // Call and discard. React.lazy() will call it again and use the result or the error. + // This is fine because import() does not re-fetch/re-run the module. + try { + await importFunction(); + } catch { + // React.lazy() will take care of it + } finally { + observer.disconnect(); } +} + +// Wrap React.lazy() to isolate the component in a shadow DOM +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- It must be `any` +export function reactLazyIsolatedComponent>( + importFunction: LazyComponentImportFunction, +): React.LazyExoticComponent { + void discardStylesheetsWhilePending(importFunction); - return false; + return React.lazy(importFunction) as React.LazyExoticComponent; } /** @@ -73,32 +107,17 @@ export const IsolatedComponent: React.VFC<{ */ children: JSX.Element; }> = ({ webpackChunkName, children, noStyle, ...props }) => { + if ( + !isolatedComponentList.some((url) => url.endsWith("/" + webpackChunkName)) + ) { + throw new Error( + `Isolated component "${webpackChunkName}" is not listed in isolatedComponentList.mjs. Add it there and restart webpack to create it.`, + ); + } + const stylesheetUrl = noStyle ? null - : chrome.runtime.getURL(`css/${webpackChunkName}.css`); - - // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. - useEffect(() => { - if (!stylesheetUrl) { - // This component doesn't generate any stylesheets - return; - } - - if (deactivateGlobalStyle(stylesheetUrl)) { - // This stylesheet is injected only once per document, don't await further injections. - return; - } - - const observer = new MutationObserver(() => { - if (deactivateGlobalStyle(stylesheetUrl)) { - observer.disconnect(); - } - }); - observer.observe(document.head, { childList: true }); - return () => { - observer.disconnect(); - }; - }); + : chrome.runtime.getURL(`${webpackChunkName}.css`); return ( // `pb-name` is used to visually identify it in the dev tools diff --git a/src/components/isolatedComponentList.d.ts b/src/components/isolatedComponentList.d.ts new file mode 100644 index 0000000000..f1c0a4afa4 --- /dev/null +++ b/src/components/isolatedComponentList.d.ts @@ -0,0 +1,3 @@ +const list: string[]; + +export default list; diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs new file mode 100644 index 0000000000..42ac352024 --- /dev/null +++ b/src/components/isolatedComponentList.mjs @@ -0,0 +1,37 @@ +/* + * 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 . + */ + +/** + * @file Read by: + * - Webpack to create individual bundles + * - IsolatedComponent.tsx to ensure the `{webpackChunkName}.css` file will exist + * - ESLint to validate the usage of `webpackChunkName` + */ + +// These URLs are not automatically updated when refactoring. +// TODO: Find a format supported by IDEs. https://github.com/microsoft/TypeScript/issues/43759#issuecomment-2041000482 + +// Path rules: +// - Use relative paths from the `src` directory +// - Do not start with `/` +// - Do not add the file extension +const components = [ + "bricks/renderers/PropertyTree", + "components/selectionToolPopover/SelectionToolPopover", +]; + +export default components; diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx index 8424d87db1..7400a0a3d4 100644 --- a/src/components/selectionToolPopover/showSelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -22,7 +22,10 @@ import React from "react"; import { render } from "react-dom"; -import { IsolatedComponent } from "@/components/IsolatedComponent"; +import { + IsolatedComponent, + reactLazyIsolatedComponent, +} from "@/components/IsolatedComponent"; import type Component from "@/components/selectionToolPopover/SelectionToolPopover"; export default function showSelectionToolPopover({ @@ -31,10 +34,10 @@ export default function showSelectionToolPopover({ }: { rootElement: HTMLElement; } & React.ComponentProps) { - const SelectionToolPopover = React.lazy( + const SelectionToolPopover = reactLazyIsolatedComponent( async () => import( - /* webpackChunkName: "SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" + /* webpackChunkName: "isolated/SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" ), ); diff --git a/webpack.config.mjs b/webpack.config.mjs index 217515075d..9242168739 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -30,11 +30,11 @@ import { compact } from "lodash-es"; import mergeWithShared from "./webpack.sharedConfig.js"; import { parseEnv, loadEnv } from "./scripts/env.mjs"; import customizeManifest from "./scripts/manifest.mjs"; -import { createRequire } from "node:module"; import DiscardFilePlugin from "./scripts/DiscardFilePlugin.mjs"; +import isolatedComponentList from "./src/components/isolatedComponentList.mjs"; import ReactRefreshWebpackPlugin from "@pmmmwh/react-refresh-webpack-plugin"; - -const require = createRequire(import.meta.url); +import bootstrapIconsPackage from "bootstrap-icons/package.json" with { type: "json" }; +import simpleIconsPackage from "simple-icons/package.json" with { type: "json" }; loadEnv(); @@ -48,11 +48,9 @@ console.log( process.env.REQUIRE_OPTIONAL_PERMISSIONS_IN_MANIFEST, ); -if (!process.env.SOURCE_VERSION) { - process.env.SOURCE_VERSION = execSync("git rev-parse --short HEAD") - .toString() - .trim(); -} +process.env.SOURCE_VERSION ??= execSync("git rev-parse --short HEAD") + .toString() + .trim(); // Configure sourcemaps // Disable sourcemaps on CI unless it's a PUBLIC_RELEASE @@ -113,19 +111,17 @@ const createConfig = (env, options) => entry: Object.fromEntries( [ "background/background", - // Components rendered within the Shadow DOM, such as those used by the Document Renderer brick in the sidebar, - // are isolated from global styles. This prevents access to CSS module classes used by these components. - // To resolve this, add the root component of the affected component hierarchy to the webpack function that - // bundles styles. This will make the CSS available to be loaded by the component tree. - // Additionally, remember to add the related JavaScript file to the DiscardFilePlugin.mjs file to exclude - // it from the bundle, as it is not needed for rendering in this context. + + // TODO: Move to isolatedComponentList.mjs and use "bricks/renderers/CustomFormComponent", "bricks/renderers/documentView/DocumentView", "bricks/transformers/ephemeralForm/EphemeralFormContent", + "contentScript/contentScript", "contentScript/loadActivationEnhancements", "contentScript/browserActionInstantHandler", "contentScript/setExtensionIdInApp", + "pageEditor/pageEditor", "extensionConsole/options", "sidebar/sidebar", @@ -142,6 +138,9 @@ const createConfig = (env, options) => // The script that gets injected into the host page "pageScript/pageScript", + + // The isolated components whose CSS will be loaded in a shadow DOM + ...isolatedComponentList, ].map((name) => [path.basename(name), `./src/${name}`]), ), @@ -306,7 +305,7 @@ const createConfig = (env, options) => rules: [ { test: /\.s?css$/, - resourceQuery: { not: [/loadAsUrl/] }, + resourceQuery: { not: [/loadAsUrl|loadAsText/] }, use: [MiniCssExtractPlugin.loader, "css-loader"], }, { @@ -329,9 +328,7 @@ const createConfig = (env, options) => type: "asset/resource", generator: { emit: false, - publicPath: `https://cdn.jsdelivr.net/npm/bootstrap-icons@${ - require("bootstrap-icons/package.json").version - }/`, + publicPath: `https://cdn.jsdelivr.net/npm/bootstrap-icons@${bootstrapIconsPackage.version}/`, filename: "icons/[name][ext]", }, }, @@ -340,9 +337,7 @@ const createConfig = (env, options) => type: "asset/resource", generator: { emit: false, - publicPath: `https://cdn.jsdelivr.net/npm/simple-icons@${ - require("simple-icons/package.json").version - }/`, + publicPath: `https://cdn.jsdelivr.net/npm/simple-icons@${simpleIconsPackage.version}/`, filename: "icons/[name][ext]", }, }, diff --git a/webpack.sharedConfig.js b/webpack.sharedConfig.js index dc362ee0e8..0c4733182e 100644 --- a/webpack.sharedConfig.js +++ b/webpack.sharedConfig.js @@ -49,7 +49,7 @@ const shared = { // Lighter jQuery version jquery: "jquery/dist/jquery.slim.min.js", }, - extensions: [".ts", ".tsx", ".jsx", ".js"], + extensions: [".ts", ".tsx", ".jsx", ".js", ".mjs"], fallback: { fs: false, crypto: false, From a6dc0fed1cf3a32d865e2177edd26a4f3dedbf14 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 16:50:33 +0700 Subject: [PATCH 12/33] Ok with fonts --- src/components/IsolatedComponent.scss | 17 +---------- src/components/IsolatedComponent.tsx | 2 ++ src/components/Stylesheets.tsx | 28 +++++++++++++++++++ .../SelectionToolPopover.stories.tsx | 4 --- 4 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/components/IsolatedComponent.scss b/src/components/IsolatedComponent.scss index 0a06be0df8..09ee9fe6e0 100644 --- a/src/components/IsolatedComponent.scss +++ b/src/components/IsolatedComponent.scss @@ -1,19 +1,4 @@ -/* - * 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 . - */ +// Injected unminified. Don't add too much fluff :host { // Don't inherit any style diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 61fd0ab807..b8e3adfc77 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -31,6 +31,7 @@ type LazyComponentImportFunction = () => Promise<{ }>; // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. +// Until this is resolved https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 async function discardStylesheetsWhilePending( importFunction: LazyComponentImportFunction, ) { @@ -43,6 +44,7 @@ async function discardStylesheetsWhilePending( if (node instanceof HTMLLinkElement && node.href.startsWith(baseUrl)) { // Disable stylesheet without removing it. Webpack still awaits its loading. node.media = "not all"; + node.dataset.pixiebrix = "Disabled by IsolatedComponent"; } } } diff --git a/src/components/Stylesheets.tsx b/src/components/Stylesheets.tsx index 2948aa72ca..f3c66556d1 100644 --- a/src/components/Stylesheets.tsx +++ b/src/components/Stylesheets.tsx @@ -17,6 +17,8 @@ import React, { useState } from "react"; import { castArray, uniq } from "lodash"; +import oneEvent from "one-event"; +import { assertNotNullish } from "@/utils/nullishUtils"; /** * Loads one or more stylesheets and hides the content until they're done loading. @@ -53,6 +55,32 @@ export const Stylesheets: React.FC<{ return ( { + // Detect and extract font-face rules because they're + // not loaded from the shadow DOM's stylesheets + // https://issues.chromium.org/issues/41085401 + const isShadowRoot = link?.getRootNode() instanceof ShadowRoot; + if (!isShadowRoot) { + return; + } + + if (!link.sheet) { + await oneEvent(link, "load"); + assertNotNullish( + link.sheet, + "The stylesheet wasn't parsed after loading", + ); + } + + const fontFaceStylesheet = new CSSStyleSheet(); + for (const rule of link.sheet.cssRules) { + if (rule instanceof CSSFontFaceRule) { + fontFaceStylesheet.insertRule(rule.cssText); + } + } + + document.adoptedStyleSheets.push(fontFaceStylesheet); + }} href={href} key={href} onLoad={resolve} diff --git a/src/components/selectionToolPopover/SelectionToolPopover.stories.tsx b/src/components/selectionToolPopover/SelectionToolPopover.stories.tsx index fdf71bfc21..4f19e9014d 100644 --- a/src/components/selectionToolPopover/SelectionToolPopover.stories.tsx +++ b/src/components/selectionToolPopover/SelectionToolPopover.stories.tsx @@ -19,10 +19,6 @@ import React from "react"; import { type ComponentStory, type ComponentMeta } from "@storybook/react"; import SelectionToolPopover from "@/components/selectionToolPopover/SelectionToolPopover"; -/* - * This Storybook component is unstyled because it's wrapped inside the react-shadow-dom. - * It loads styles as url and inject as - {children} + {factory(LazyComponent)} ); -}; +} diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx index 7400a0a3d4..742857919b 100644 --- a/src/components/selectionToolPopover/showSelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -22,10 +22,7 @@ import React from "react"; import { render } from "react-dom"; -import { - IsolatedComponent, - reactLazyIsolatedComponent, -} from "@/components/IsolatedComponent"; +import IsolatedComponent from "@/components/IsolatedComponent"; import type Component from "@/components/selectionToolPopover/SelectionToolPopover"; export default function showSelectionToolPopover({ @@ -34,17 +31,17 @@ export default function showSelectionToolPopover({ }: { rootElement: HTMLElement; } & React.ComponentProps) { - const SelectionToolPopover = reactLazyIsolatedComponent( - async () => - import( - /* webpackChunkName: "isolated/SelectionToolPopover" */ "@/components/selectionToolPopover/SelectionToolPopover" - ), - ); - render( - - - , + + import( + /* webpackChunkName: "isolated/SelectionToolPopover" */ + "@/components/selectionToolPopover/SelectionToolPopover" + ) + } + factory={(SelectionToolPopover) => } + >, rootElement, ); } diff --git a/src/contentScript/textSelectionMenu/selectionMenuController.tsx b/src/contentScript/textSelectionMenu/selectionMenuController.tsx index 9c13b8f3aa..5e2cf48407 100644 --- a/src/contentScript/textSelectionMenu/selectionMenuController.tsx +++ b/src/contentScript/textSelectionMenu/selectionMenuController.tsx @@ -39,7 +39,7 @@ import { getSelectionRange } from "@/utils/domUtils"; import { snapWithin } from "@/utils/mathUtils"; import ActionRegistry from "@/contentScript/textSelectionMenu/ActionRegistry"; import { SELECTION_MENU_READY_ATTRIBUTE } from "@/domConstants"; -import { IsolatedComponent } from "@/components/IsolatedComponent"; +import IsolatedComponent from "@/components/IsolatedComponent"; const MIN_SELECTION_LENGTH_CHARS = 3; @@ -164,21 +164,22 @@ function createSelectionMenu(): HTMLElement { selectionMenu = tooltipFactory(); selectionMenu.dataset.testid = "pixiebrix-selection-menu"; - const TextSelectionMenu = React.lazy( - async () => - import( - /* webpackChunkName: "SelectionMenu" */ - "@/contentScript/textSelectionMenu/SelectionMenu" - ), - ); - render( - - - , + + import( + /* webpackChunkName: "SelectionMenu" */ + "@/contentScript/textSelectionMenu/SelectionMenu" + ) + } + factory={(SelectionMenu) => ( + + )} + />, selectionMenu, ); From 123b53a6bf2ceec06e893c467b1c2c8201743782 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 18:14:07 +0700 Subject: [PATCH 16/33] Fix most issues --- scripts/__snapshots__/manifest.test.js.snap | 24 +++---------------- src/components/Stylesheets.test.tsx | 21 +++++++++------- src/components/isolatedComponentList.mjs | 1 + .../SelectionToolPopover.module.scss | 2 ++ .../selectionMenuController.tsx | 2 +- src/manifest.json | 10 ++------ 6 files changed, 22 insertions(+), 38 deletions(-) diff --git a/scripts/__snapshots__/manifest.test.js.snap b/scripts/__snapshots__/manifest.test.js.snap index 6f21033674..39e7742989 100644 --- a/scripts/__snapshots__/manifest.test.js.snap +++ b/scripts/__snapshots__/manifest.test.js.snap @@ -133,21 +133,15 @@ exports[`customizeManifest beta 1`] = ` "bundles/*", "sandbox.html", "frame.html", - "frame.css", "sidebar.html", - "sidebar.css", - "pageEditor.css", "pageScript.js", "ephemeralForm.html", "walkthroughModal.html", "ephemeralPanel.html", - "ephemeralModal.css", - "DocumentView.css", - "CustomFormComponent.css", - "EphemeralFormContent.css", "audio/*", "user-icons/*", "img/*", + "*.css", ], }, ], @@ -273,21 +267,15 @@ exports[`customizeManifest mv2 1`] = ` "bundles/*", "sandbox.html", "frame.html", - "frame.css", "sidebar.html", - "sidebar.css", - "pageEditor.css", "pageScript.js", "ephemeralForm.html", "walkthroughModal.html", "ephemeralPanel.html", - "ephemeralModal.css", - "DocumentView.css", - "CustomFormComponent.css", - "EphemeralFormContent.css", "audio/*", "user-icons/*", "img/*", + "*.css", ], } `; @@ -425,21 +413,15 @@ exports[`customizeManifest mv3 1`] = ` "bundles/*", "sandbox.html", "frame.html", - "frame.css", "sidebar.html", - "sidebar.css", - "pageEditor.css", "pageScript.js", "ephemeralForm.html", "walkthroughModal.html", "ephemeralPanel.html", - "ephemeralModal.css", - "DocumentView.css", - "CustomFormComponent.css", - "EphemeralFormContent.css", "audio/*", "user-icons/*", "img/*", + "*.css", ], }, ], diff --git a/src/components/Stylesheets.test.tsx b/src/components/Stylesheets.test.tsx index 21478a64b5..cd8944813b 100644 --- a/src/components/Stylesheets.test.tsx +++ b/src/components/Stylesheets.test.tsx @@ -36,8 +36,8 @@ it("renders the children after the stylesheet is loaded", () => { , ); - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element - const stylesheet = container.querySelector("link"); + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access, @typescript-eslint/no-unnecessary-type-assertion -- Not a visible element + const stylesheet = container.querySelector("link")!; expect(stylesheet).toHaveAttribute("href", "https://example.com/style.css"); // It should be in the document but not visible @@ -65,19 +65,23 @@ it("renders the children after the stylesheet is loaded when multiple stylesheet // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element const stylesheets = container.querySelectorAll("link"); expect(stylesheets).toHaveLength(2); - expect(stylesheets[0]).toHaveAttribute( + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length + expect(stylesheets[0]!).toHaveAttribute( "href", "https://example.com/style1.css", ); - expect(stylesheets[1]).toHaveAttribute( + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length + expect(stylesheets[1]!).toHaveAttribute( "href", "https://example.com/style2.css", ); expect(screen.queryByRole("button")).not.toBeInTheDocument(); - fireEvent.load(stylesheets[0]); - fireEvent.load(stylesheets[1]); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length + fireEvent.load(stylesheets[0]!); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length + fireEvent.load(stylesheets[1]!); // Now it should be visible expect(screen.getByRole("button")).toBeInTheDocument(); @@ -90,8 +94,9 @@ it("renders the children immediately even when the loading of the stylesheet fai , ); - // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element - const stylesheet = container.querySelector("link"); + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access, @typescript-eslint/no-unnecessary-type-assertion -- Not a visible element + const stylesheet = container.querySelector("link")!; + expect(stylesheet).not.toBeNull(); expect(stylesheet).toHaveAttribute("href", "https://example.com/style.css"); expect(screen.queryByRole("button")).not.toBeInTheDocument(); diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs index 42ac352024..d6e7a818f9 100644 --- a/src/components/isolatedComponentList.mjs +++ b/src/components/isolatedComponentList.mjs @@ -32,6 +32,7 @@ const components = [ "bricks/renderers/PropertyTree", "components/selectionToolPopover/SelectionToolPopover", + "contentScript/textSelectionMenu/SelectionMenu", ]; export default components; diff --git a/src/components/selectionToolPopover/SelectionToolPopover.module.scss b/src/components/selectionToolPopover/SelectionToolPopover.module.scss index 631c893da3..8f478dc4f2 100644 --- a/src/components/selectionToolPopover/SelectionToolPopover.module.scss +++ b/src/components/selectionToolPopover/SelectionToolPopover.module.scss @@ -44,5 +44,7 @@ $border: rgba(0, 0, 0, 0.3); } .body { + display: flex; + align-items: center; padding: 12px; } diff --git a/src/contentScript/textSelectionMenu/selectionMenuController.tsx b/src/contentScript/textSelectionMenu/selectionMenuController.tsx index 5e2cf48407..9b322f4bf3 100644 --- a/src/contentScript/textSelectionMenu/selectionMenuController.tsx +++ b/src/contentScript/textSelectionMenu/selectionMenuController.tsx @@ -169,7 +169,7 @@ function createSelectionMenu(): HTMLElement { webpackChunkName="SelectionMenu" lazy={async () => import( - /* webpackChunkName: "SelectionMenu" */ + /* webpackChunkName: "isolated/SelectionMenu" */ "@/contentScript/textSelectionMenu/SelectionMenu" ) } diff --git a/src/manifest.json b/src/manifest.json index f28355735b..89dad77a48 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -66,21 +66,15 @@ "bundles/*", "sandbox.html", "frame.html", - "frame.css", "sidebar.html", - "sidebar.css", - "pageEditor.css", "pageScript.js", "ephemeralForm.html", "walkthroughModal.html", "ephemeralPanel.html", - "ephemeralModal.css", - "DocumentView.css", - "CustomFormComponent.css", - "EphemeralFormContent.css", "audio/*", "user-icons/*", - "img/*" + "img/*", + "*.css" ], "browser_action": { "default_title": "PixieBrix", From fb4d8a4c53dc8bcc06515d565148964e2709750d Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 18:46:34 +0700 Subject: [PATCH 17/33] Extract/cleanup --- src/bricks/renderers/propertyTable.tsx | 2 +- src/components/InvalidatedContextGate.tsx | 5 +- src/components/IsolatedComponent.tsx | 48 ++++++++----------- src/components/Stylesheets.test.tsx | 17 ++++--- src/components/isolatedComponentList.mjs | 4 +- src/components/jsonTree/treeHooks.tsx | 2 +- .../showSelectionToolPopover.tsx | 7 +-- .../selectionMenuController.tsx | 2 +- webpack.config.mjs | 21 +++++--- 9 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/bricks/renderers/propertyTable.tsx b/src/bricks/renderers/propertyTable.tsx index 08a16b1ed1..7930f1e41b 100644 --- a/src/bricks/renderers/propertyTable.tsx +++ b/src/bricks/renderers/propertyTable.tsx @@ -119,7 +119,7 @@ export class PropertyTableRenderer extends RendererABC { async render({ data }: BrickArgs, { ctxt }: BrickOptions) { const PropertyTree: React.FC<{ value: TreeNode[] }> = ({ value }) => ( import( /* webpackChunkName: "isolated/PropertyTree" */ diff --git a/src/components/InvalidatedContextGate.tsx b/src/components/InvalidatedContextGate.tsx index 9b4353f75e..4903711e73 100644 --- a/src/components/InvalidatedContextGate.tsx +++ b/src/components/InvalidatedContextGate.tsx @@ -28,10 +28,7 @@ const InvalidatedContextGate: React.FunctionComponent<{ // Only auto-reload if the document is in the background const isDocumentVisible = useDocumentVisibility(); if (wasContextInvalidated && autoReload && !isDocumentVisible) { - setTimeout(() => { - // If you reload too soon, Chrome might not be ready to serve the page yet - location.reload(); - }, 500); + location.reload(); } return wasContextInvalidated ? ( diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 8115da6c54..7f5261d26c 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -67,64 +67,58 @@ async function discardStylesheetsWhilePending( type Props = { /** - * It must match the `webpackChunkName` specified in the React.lazy import + * It must match the `import()`ed component's filename */ - webpackChunkName: string; + name: string; /** - * @example () => import(/* webpackChunkName: "Moon" * /, "@/components/Moon") + * It must follow the format `isolated/${name}` specified above + * @example () => import(/* webpackChunkName: "isolated/Moon" * /, "@/components/Moon") */ lazy: LazyFactory; - /** - * If true, the component will not attempt to load the stylesheet. - * - * @example - */ - noStyle?: boolean; - /** * It must be the result of React.lazy() */ factory: ( Component: React.LazyExoticComponent>, ) => JSX.Element; + + /** + * If true, the component will not attempt to load the stylesheet. + * + * @example + */ + noStyle?: boolean; }; /** * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. * * @example - * const Moon = React.lazy(() => import( - * /* webpackChunkName: Moon / - * "./Moon" - * )); - * * render( - * - * - * , + * import(/* webpackChunkName: "isolated/Moon" * / "@/components/Moon")} + * factory={(Moon) => } + * />, * document.querySelector('#container'), * ); */ export default function IsolatedComponent({ - webpackChunkName, + name, factory, noStyle, lazy, ...props }: Props) { - if ( - !isolatedComponentList.some((url) => url.endsWith("/" + webpackChunkName)) - ) { + if (!isolatedComponentList.some((url) => url.endsWith("/" + name))) { throw new Error( - `Isolated component "${webpackChunkName}" is not listed in isolatedComponentList.mjs. Add it there and restart webpack to create it.`, + `Isolated component "${name}" is not listed in isolatedComponentList.mjs. Add it there and restart webpack to create it.`, ); } - const stylesheetUrl = noStyle - ? null - : chrome.runtime.getURL(`${webpackChunkName}.css`); + const stylesheetUrl = noStyle ? null : chrome.runtime.getURL(`${name}.css`); // `discard` one must be called before `React.lazy` void discardStylesheetsWhilePending(lazy); @@ -132,7 +126,7 @@ export default function IsolatedComponent({ return ( // `pb-name` is used to visually identify it in the dev tools - + {factory(LazyComponent)} diff --git a/src/components/Stylesheets.test.tsx b/src/components/Stylesheets.test.tsx index cd8944813b..a8ea91a980 100644 --- a/src/components/Stylesheets.test.tsx +++ b/src/components/Stylesheets.test.tsx @@ -63,25 +63,24 @@ it("renders the children after the stylesheet is loaded when multiple stylesheet ); // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- Not a visible element - const stylesheets = container.querySelectorAll("link"); + const stylesheets = container.querySelectorAll("link") as unknown as [ + HTMLLinkElement, + HTMLLinkElement, + ]; expect(stylesheets).toHaveLength(2); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length - expect(stylesheets[0]!).toHaveAttribute( + expect(stylesheets[0]).toHaveAttribute( "href", "https://example.com/style1.css", ); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length - expect(stylesheets[1]!).toHaveAttribute( + expect(stylesheets[1]).toHaveAttribute( "href", "https://example.com/style2.css", ); expect(screen.queryByRole("button")).not.toBeInTheDocument(); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length - fireEvent.load(stylesheets[0]!); - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- Asserted via length - fireEvent.load(stylesheets[1]!); + fireEvent.load(stylesheets[0]); + fireEvent.load(stylesheets[1]); // Now it should be visible expect(screen.getByRole("button")).toBeInTheDocument(); diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs index d6e7a818f9..16efc8f420 100644 --- a/src/components/isolatedComponentList.mjs +++ b/src/components/isolatedComponentList.mjs @@ -29,10 +29,10 @@ // - Use relative paths from the `src` directory // - Do not start with `/` // - Do not add the file extension -const components = [ +const isolatedComponentList = [ "bricks/renderers/PropertyTree", "components/selectionToolPopover/SelectionToolPopover", "contentScript/textSelectionMenu/SelectionMenu", ]; -export default components; +export default isolatedComponentList; diff --git a/src/components/jsonTree/treeHooks.tsx b/src/components/jsonTree/treeHooks.tsx index 5c9ed9ac57..719f5f111e 100644 --- a/src/components/jsonTree/treeHooks.tsx +++ b/src/components/jsonTree/treeHooks.tsx @@ -42,7 +42,7 @@ export function useLabelRenderer() { href="#" onClick={async (event) => { await writeToClipboard({ - text: getPathFromArray([...keyPath].reverse()), + text: getPathFromArray(keyPath.reverse()), }); event.preventDefault(); event.stopPropagation(); diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx index 742857919b..1b0c180ff4 100644 --- a/src/components/selectionToolPopover/showSelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -15,11 +15,6 @@ * along with this program. If not, see . */ -/** - * @file This file exists to facilitate testing/mocking. The function cannot live - * in SelectionToolPopover.tsx because it must import it asynchronously. - */ - import React from "react"; import { render } from "react-dom"; import IsolatedComponent from "@/components/IsolatedComponent"; @@ -33,7 +28,7 @@ export default function showSelectionToolPopover({ } & React.ComponentProps) { render( import( /* webpackChunkName: "isolated/SelectionToolPopover" */ diff --git a/src/contentScript/textSelectionMenu/selectionMenuController.tsx b/src/contentScript/textSelectionMenu/selectionMenuController.tsx index 9b322f4bf3..1e7a4e3266 100644 --- a/src/contentScript/textSelectionMenu/selectionMenuController.tsx +++ b/src/contentScript/textSelectionMenu/selectionMenuController.tsx @@ -166,7 +166,7 @@ function createSelectionMenu(): HTMLElement { render( import( /* webpackChunkName: "isolated/SelectionMenu" */ diff --git a/webpack.config.mjs b/webpack.config.mjs index 6e04eb1f09..e0711dc698 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -30,11 +30,12 @@ import { compact } from "lodash-es"; import mergeWithShared from "./webpack.sharedConfig.js"; import { parseEnv, loadEnv } from "./scripts/env.mjs"; import customizeManifest from "./scripts/manifest.mjs"; +import { createRequire } from "node:module"; import DiscardFilePlugin from "./scripts/DiscardFilePlugin.mjs"; import isolatedComponentList from "./src/components/isolatedComponentList.mjs"; import ReactRefreshWebpackPlugin from "@pmmmwh/react-refresh-webpack-plugin"; -import bootstrapIconsPackage from "bootstrap-icons/package.json" assert { type: "json" }; -import simpleIconsPackage from "simple-icons/package.json" assert { type: "json" }; + +const require = createRequire(import.meta.url); loadEnv(); @@ -48,9 +49,11 @@ console.log( process.env.REQUIRE_OPTIONAL_PERMISSIONS_IN_MANIFEST, ); -process.env.SOURCE_VERSION ??= execSync("git rev-parse --short HEAD") - .toString() - .trim(); +if (!process.env.SOURCE_VERSION) { + process.env.SOURCE_VERSION = execSync("git rev-parse --short HEAD") + .toString() + .trim(); +} // Configure sourcemaps // Disable sourcemaps on CI unless it's a PUBLIC_RELEASE @@ -328,7 +331,9 @@ const createConfig = (env, options) => type: "asset/resource", generator: { emit: false, - publicPath: `https://cdn.jsdelivr.net/npm/bootstrap-icons@${bootstrapIconsPackage.version}/`, + publicPath: `https://cdn.jsdelivr.net/npm/bootstrap-icons@${ + require("bootstrap-icons/package.json").version + }/`, filename: "icons/[name][ext]", }, }, @@ -337,7 +342,9 @@ const createConfig = (env, options) => type: "asset/resource", generator: { emit: false, - publicPath: `https://cdn.jsdelivr.net/npm/simple-icons@${simpleIconsPackage.version}/`, + publicPath: `https://cdn.jsdelivr.net/npm/simple-icons@${ + require("simple-icons/package.json").version + }/`, filename: "icons/[name][ext]", }, }, From a349a66aa69c741e76127189cf5bcaec9ae39f86 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 19:01:07 +0700 Subject: [PATCH 18/33] /2 --- src/components/Stylesheets.tsx | 55 +++++++++++++++++----------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/components/Stylesheets.tsx b/src/components/Stylesheets.tsx index f3c66556d1..5328a29e59 100644 --- a/src/components/Stylesheets.tsx +++ b/src/components/Stylesheets.tsx @@ -20,6 +20,33 @@ import { castArray, uniq } from "lodash"; import oneEvent from "one-event"; import { assertNotNullish } from "@/utils/nullishUtils"; +/** + * Detect and extract font-face rules because Chrome fails to load them from + * the shadow DOM's stylesheets: https://issues.chromium.org/issues/41085401 + */ +async function extractFontFaceRulesToMainDocument( + link: HTMLLinkElement | null, +) { + const isShadowRoot = link?.getRootNode() instanceof ShadowRoot; + if (!isShadowRoot) { + return; + } + + if (!link.sheet) { + await oneEvent(link, "load"); + assertNotNullish(link.sheet, "The stylesheet wasn't parsed after loading"); + } + + const fontFaceStylesheet = new CSSStyleSheet(); + for (const rule of link.sheet.cssRules) { + if (rule instanceof CSSFontFaceRule) { + fontFaceStylesheet.insertRule(rule.cssText); + } + } + + document.adoptedStyleSheets.push(fontFaceStylesheet); +} + /** * Loads one or more stylesheets and hides the content until they're done loading. * @@ -41,7 +68,6 @@ export const Stylesheets: React.FC<{ return <>{children}; } - // `every` returns true for empty arrays const urls = uniq(castArray(href)); const allResolved = urls.every((url) => resolved.includes(url)); @@ -55,32 +81,7 @@ export const Stylesheets: React.FC<{ return ( { - // Detect and extract font-face rules because they're - // not loaded from the shadow DOM's stylesheets - // https://issues.chromium.org/issues/41085401 - const isShadowRoot = link?.getRootNode() instanceof ShadowRoot; - if (!isShadowRoot) { - return; - } - - if (!link.sheet) { - await oneEvent(link, "load"); - assertNotNullish( - link.sheet, - "The stylesheet wasn't parsed after loading", - ); - } - - const fontFaceStylesheet = new CSSStyleSheet(); - for (const rule of link.sheet.cssRules) { - if (rule instanceof CSSFontFaceRule) { - fontFaceStylesheet.insertRule(rule.cssText); - } - } - - document.adoptedStyleSheets.push(fontFaceStylesheet); - }} + ref={extractFontFaceRulesToMainDocument} href={href} key={href} onLoad={resolve} From efb53fe6afd5ef15b0b4e361c7236891308a4e21 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Sat, 6 Apr 2024 19:48:22 +0700 Subject: [PATCH 19/33] Move DiscardFilePlugin config out --- scripts/DiscardFilePlugin.mjs | 13 ++++++------- webpack.config.mjs | 6 +++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts/DiscardFilePlugin.mjs b/scripts/DiscardFilePlugin.mjs index a7fa5d1619..322dfc75a8 100644 --- a/scripts/DiscardFilePlugin.mjs +++ b/scripts/DiscardFilePlugin.mjs @@ -1,9 +1,11 @@ import webpack from "webpack"; -// eslint-disable-next-line no-restricted-imports -- TODO: Rule should not apply here -import isolatedComponentList from "../src/components/isolatedComponentList.mjs"; // https://github.com/pixiebrix/pixiebrix-extension/pull/7363#discussion_r1458224740 export default class DiscardFilePlugin { + constructor(filesToDiscard) { + this.filesToDiscard = filesToDiscard; + } + apply(compiler) { compiler.hooks.compilation.tap("DiscardFilePlugin", (compilation) => { compilation.hooks.processAssets.tapPromise( @@ -12,16 +14,13 @@ export default class DiscardFilePlugin { stage: webpack.Compilation.PROCESS_ASSETS_STAGE_OPTIMIZE_SIZE, }, async (assets) => { - // These files are not used, they're only webpack entry points in order to generate - // a full CSS files that can be injected in shadow DOM. See this for more context: - // https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 - for (const componentPath of isolatedComponentList) { + for (const componentPath of this.filesToDiscard) { delete assets[`${componentPath.split("/").pop()}.js`]; // If `delete assets[]` causes issues in the future, try replacing the content instead: // assets["DocumentView.js"] = new webpack.sources.RawSource('"Dropped"'); } - // TODO: Remove these 3 from here and use + // TODO: Use and move these to isolatedComponentList delete assets["DocumentView.js"]; delete assets["EphemeralFormContent.js"]; delete assets["CustomFormComponent.js"]; diff --git a/webpack.config.mjs b/webpack.config.mjs index e0711dc698..d8ab78a482 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -297,7 +297,11 @@ const createConfig = (env, options) => "static", ], }), - new DiscardFilePlugin(), + + // These files are not used, they're only webpack entry points in order to generate + // a full CSS files that can be injected in shadow DOM. See this for more context: + // https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 + new DiscardFilePlugin(isolatedComponentList), isHMR && new ReactRefreshWebpackPlugin({ From f195c5c07d2539c0adb517371175bb7b2da7fd63 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 20:05:59 +0700 Subject: [PATCH 20/33] Update documentation --- src/components/IsolatedComponent.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 7f5261d26c..50825921c5 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -78,7 +78,8 @@ type Props = { lazy: LazyFactory; /** - * It must be the result of React.lazy() + * A function that provides the loaded component as an argument, and calls it + * @example (Moon) => */ factory: ( Component: React.LazyExoticComponent>, @@ -100,7 +101,7 @@ type Props = { * import(/* webpackChunkName: "isolated/Moon" * / "@/components/Moon")} - * factory={(Moon) => } + * factory={(Moon) => } * />, * document.querySelector('#container'), * ); From ccb84c2d8300816b5d2a56e52352aa597d9f9b38 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 20:15:01 +0700 Subject: [PATCH 21/33] CustomFormComponent --- scripts/DiscardFilePlugin.mjs | 1 - src/bricks/renderers/CustomFormComponent.tsx | 21 +++++++++---------- .../{customForm.ts => customForm.tsx} | 21 +++++++++++++++---- src/components/Stylesheets.tsx | 4 ++-- src/components/StylesheetsContext.ts | 7 ++----- src/components/isolatedComponentList.mjs | 1 + .../sidebar/sidebarBootstrapOverrides.scss | 0 webpack.config.mjs | 1 - 8 files changed, 32 insertions(+), 24 deletions(-) rename src/bricks/renderers/{customForm.ts => customForm.tsx} (95%) rename src/{pageEditor => }/sidebar/sidebarBootstrapOverrides.scss (100%) diff --git a/scripts/DiscardFilePlugin.mjs b/scripts/DiscardFilePlugin.mjs index 322dfc75a8..cf99410e1e 100644 --- a/scripts/DiscardFilePlugin.mjs +++ b/scripts/DiscardFilePlugin.mjs @@ -23,7 +23,6 @@ export default class DiscardFilePlugin { // TODO: Use and move these to isolatedComponentList delete assets["DocumentView.js"]; delete assets["EphemeralFormContent.js"]; - delete assets["CustomFormComponent.js"]; }, ); }); diff --git a/src/bricks/renderers/CustomFormComponent.tsx b/src/bricks/renderers/CustomFormComponent.tsx index 8551f80228..4f45f6af9e 100644 --- a/src/bricks/renderers/CustomFormComponent.tsx +++ b/src/bricks/renderers/CustomFormComponent.tsx @@ -15,6 +15,9 @@ * along with this program. If not, see . */ +import "@/vendors/bootstrapWithoutRem.css"; +import "@/sidebar/sidebarBootstrapOverrides.scss"; +import "@/bricks/renderers/customForm.css"; import React, { useEffect, useRef, useState } from "react"; import { type Schema, type UiSchema } from "@/types/schemaTypes"; import { type JsonObject } from "type-fest"; @@ -31,7 +34,6 @@ import TextAreaWidget from "@/components/formBuilder/TextAreaWidget"; import RjsfSubmitContext from "@/components/formBuilder/RjsfSubmitContext"; import { templates } from "@/components/formBuilder/RjsfTemplates"; import { cloneDeep } from "lodash"; -import { useStylesheetsContextWithFormDefault } from "@/components/StylesheetsContext"; const FIELDS = { DescriptionField, @@ -43,7 +45,7 @@ const UI_WIDGETS = { TextareaWidget: TextAreaWidget, } as const; -const CustomFormComponent: React.FunctionComponent<{ +export type CustomFormComponentProps = { schema: Schema; uiSchema: UiSchema; submitCaption: string; @@ -63,8 +65,11 @@ const CustomFormComponent: React.FunctionComponent<{ resetOnSubmit?: boolean; className?: string; stylesheets?: string[]; - disableParentStyles?: boolean; -}> = ({ +}; + +const CustomFormComponent: React.FunctionComponent< + CustomFormComponentProps +> = ({ schema, uiSchema, submitCaption, @@ -73,8 +78,7 @@ const CustomFormComponent: React.FunctionComponent<{ className, onSubmit, resetOnSubmit = false, - stylesheets: newStylesheets, - disableParentStyles = false, + stylesheets, }) => { // Use useRef instead of useState because we don't need/want a re-render when count changes // This ref is used to track the onSubmit run number for runtime tracing @@ -95,11 +99,6 @@ const CustomFormComponent: React.FunctionComponent<{ setKey((prev) => prev + 1); }; - const { stylesheets } = useStylesheetsContextWithFormDefault({ - newStylesheets, - disableParentStyles, - }); - const submitData = async (data: UnknownObject): Promise => { submissionCountRef.current += 1; await onSubmit(data, { diff --git a/src/bricks/renderers/customForm.ts b/src/bricks/renderers/customForm.tsx similarity index 95% rename from src/bricks/renderers/customForm.ts rename to src/bricks/renderers/customForm.tsx index f862c019e8..d0444b88aa 100644 --- a/src/bricks/renderers/customForm.ts +++ b/src/bricks/renderers/customForm.tsx @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +import React from "react"; import { type JsonObject } from "type-fest"; import { dataStore } from "@/background/messenger/strict/api"; import { validateRegistryId } from "@/types/helpers"; @@ -46,6 +47,8 @@ import { getOutputReference, validateOutputKey } from "@/runtime/runtimeTypes"; import { type BrickConfig } from "@/bricks/types"; import { isExpression } from "@/utils/expressionUtils"; import { getPlatform } from "@/platform/platformContext"; +import IsolatedComponent from "@/components/IsolatedComponent"; +import { type CustomFormComponentProps } from "./CustomFormComponent"; interface DatabaseResult { success: boolean; @@ -297,10 +300,20 @@ export class CustomFormRenderer extends RendererABC { normalizedData, }); - // Changed webpackChunkName to de-conflict with the manual entry in webpack used to load in the stylesheets - const { default: CustomFormComponent } = await import( - /* webpackChunkName: "CustomFormRendererComponent" */ - "./CustomFormComponent" + const CustomFormComponent: React.FunctionComponent< + CustomFormComponentProps & { disableParentStyles?: boolean } + > = ({ disableParentStyles, ...props }) => ( + + import( + /* webpackChunkName: "isolated/CustomFormComponent" */ + "./CustomFormComponent" + ) + } + factory={(CustomFormComponent) => } + /> ); return { diff --git a/src/components/Stylesheets.tsx b/src/components/Stylesheets.tsx index 5328a29e59..78b384ce53 100644 --- a/src/components/Stylesheets.tsx +++ b/src/components/Stylesheets.tsx @@ -53,7 +53,7 @@ async function extractFontFaceRulesToMainDocument( * Does not support changing the initial href(s) */ export const Stylesheets: React.FC<{ - href: string | string[]; + href?: string | string[]; /** * If true, we mount the component after the stylesheets are loaded. * Chrome doesn't focus on the hidden elements so we want to make sure that component is rendered after the stylesheets are loaded. @@ -63,7 +63,7 @@ export const Stylesheets: React.FC<{ mountOnLoad?: boolean; }> = ({ href, children, mountOnLoad = false }) => { const [resolved, setResolved] = useState([]); - if (href.length === 0) { + if (!href?.length) { // Shortcut if no stylesheets are needed return <>{children}; } diff --git a/src/components/StylesheetsContext.ts b/src/components/StylesheetsContext.ts index 45839154e6..663c64500d 100644 --- a/src/components/StylesheetsContext.ts +++ b/src/components/StylesheetsContext.ts @@ -17,7 +17,7 @@ import React, { useContext } from "react"; import bootstrap from "@/vendors/bootstrapWithoutRem.css?loadAsUrl"; -import bootstrapOverrides from "@/pageEditor/sidebar/sidebarBootstrapOverrides.scss?loadAsUrl"; +import bootstrapOverrides from "@/sidebar/sidebarBootstrapOverrides.scss?loadAsUrl"; import custom from "@/bricks/renderers/customForm.css?loadAsUrl"; export type StylesheetsContextType = { @@ -76,8 +76,6 @@ export function useStylesheetsContextWithDocumentDefault({ // `webpack.config.mjs`. We build styles needed to render documents separately from the rest of the sidebar // in order to isolate the rendered document from the custom Bootstrap theme included in the Sidebar app "/DocumentView.css", - // Required because it can be nested in the DocumentView. - "/CustomFormComponent.css", ], disableParentStyles, }); @@ -97,9 +95,8 @@ export function useStylesheetsContextWithFormDefault({ defaultStylesheets: [ bootstrap, bootstrapOverrides, - // CustomFormComponent.css and EphemeralFormContent.css are artifacts produced by webpack, see the entrypoints. + // EphemeralFormContent.css is an artifact produced by webpack, see the entrypoints. "/EphemeralFormContent.css", - "/CustomFormComponent.css", custom, ], disableParentStyles, diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs index 16efc8f420..c98a57a2e7 100644 --- a/src/components/isolatedComponentList.mjs +++ b/src/components/isolatedComponentList.mjs @@ -31,6 +31,7 @@ // - Do not add the file extension const isolatedComponentList = [ "bricks/renderers/PropertyTree", + "bricks/renderers/CustomFormComponent", "components/selectionToolPopover/SelectionToolPopover", "contentScript/textSelectionMenu/SelectionMenu", ]; diff --git a/src/pageEditor/sidebar/sidebarBootstrapOverrides.scss b/src/sidebar/sidebarBootstrapOverrides.scss similarity index 100% rename from src/pageEditor/sidebar/sidebarBootstrapOverrides.scss rename to src/sidebar/sidebarBootstrapOverrides.scss diff --git a/webpack.config.mjs b/webpack.config.mjs index d8ab78a482..0ba8197088 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -116,7 +116,6 @@ const createConfig = (env, options) => "background/background", // TODO: Move to isolatedComponentList.mjs and use - "bricks/renderers/CustomFormComponent", "bricks/renderers/documentView/DocumentView", "bricks/transformers/ephemeralForm/EphemeralFormContent", From cc46b6aad2ef4eeeb9a8148e12c057b93b6b3b79 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 20:59:50 +0700 Subject: [PATCH 22/33] EphemeralFormContent --- scripts/DiscardFilePlugin.mjs | 1 - src/bricks/renderers/CustomFormComponent.tsx | 2 +- .../ephemeralForm/EphemeralForm.tsx | 34 +++++++++++++------ .../ephemeralForm/EphemeralFormContent.tsx | 34 +++++++++---------- src/components/StylesheetsContext.ts | 23 ------------- src/components/isolatedComponentList.mjs | 1 + webpack.config.mjs | 1 - 7 files changed, 42 insertions(+), 54 deletions(-) diff --git a/scripts/DiscardFilePlugin.mjs b/scripts/DiscardFilePlugin.mjs index cf99410e1e..bff8ac2198 100644 --- a/scripts/DiscardFilePlugin.mjs +++ b/scripts/DiscardFilePlugin.mjs @@ -22,7 +22,6 @@ export default class DiscardFilePlugin { // TODO: Use and move these to isolatedComponentList delete assets["DocumentView.js"]; - delete assets["EphemeralFormContent.js"]; }, ); }); diff --git a/src/bricks/renderers/CustomFormComponent.tsx b/src/bricks/renderers/CustomFormComponent.tsx index 4f45f6af9e..9d081cb6eb 100644 --- a/src/bricks/renderers/CustomFormComponent.tsx +++ b/src/bricks/renderers/CustomFormComponent.tsx @@ -27,12 +27,12 @@ import { Stylesheets } from "@/components/Stylesheets"; import JsonSchemaForm from "@rjsf/bootstrap-4"; import validator from "@/validators/formValidator"; import { type IChangeEvent } from "@rjsf/core"; +import { templates } from "@/components/formBuilder/RjsfTemplates"; import ImageCropWidget from "@/components/formBuilder/ImageCropWidget"; import RjsfSelectWidget from "@/components/formBuilder/RjsfSelectWidget"; import DescriptionField from "@/components/formBuilder/DescriptionField"; import TextAreaWidget from "@/components/formBuilder/TextAreaWidget"; import RjsfSubmitContext from "@/components/formBuilder/RjsfSubmitContext"; -import { templates } from "@/components/formBuilder/RjsfTemplates"; import { cloneDeep } from "lodash"; const FIELDS = { diff --git a/src/bricks/transformers/ephemeralForm/EphemeralForm.tsx b/src/bricks/transformers/ephemeralForm/EphemeralForm.tsx index 3c18691848..2f8b8d934f 100644 --- a/src/bricks/transformers/ephemeralForm/EphemeralForm.tsx +++ b/src/bricks/transformers/ephemeralForm/EphemeralForm.tsx @@ -26,10 +26,10 @@ import { type Target } from "@/types/messengerTypes"; import { validateUUID } from "@/types/helpers"; import { TOP_LEVEL_FRAME_ID } from "@/domConstants"; import useAsyncState from "@/hooks/useAsyncState"; -import { EphemeralFormContent } from "./EphemeralFormContent"; -import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import ErrorBoundary from "@/components/ErrorBoundary"; import useReportError from "@/hooks/useReportError"; +import IsolatedComponent from "@/components/IsolatedComponent"; +import { type EphemeralFormContentProps } from "./EphemeralFormContent"; const ModalLayout: React.FC = ({ children }) => ( // Don't use React Bootstrap's Modal because we want to customize the classes in the layout @@ -98,17 +98,31 @@ const EphemeralForm: React.FC = () => { ); } + const EphemeralFormContent: React.FunctionComponent< + EphemeralFormContentProps + > = (props) => ( + + import( + /* webpackChunkName: "isolated/EphemeralFormContent" */ + "./EphemeralFormContent" + ) + } + factory={(EphemeralFormContent) => } + /> + ); + return ( - - - + ); diff --git a/src/bricks/transformers/ephemeralForm/EphemeralFormContent.tsx b/src/bricks/transformers/ephemeralForm/EphemeralFormContent.tsx index e5166ca363..689ba4c3e4 100644 --- a/src/bricks/transformers/ephemeralForm/EphemeralFormContent.tsx +++ b/src/bricks/transformers/ephemeralForm/EphemeralFormContent.tsx @@ -15,16 +15,18 @@ * along with this program. If not, see . */ +import "@/vendors/bootstrapWithoutRem.css"; +import "@/sidebar/sidebarBootstrapOverrides.scss"; +import "@/bricks/renderers/customForm.css"; import React from "react"; import validator from "@/validators/formValidator"; import JsonSchemaForm from "@rjsf/bootstrap-4"; import { cancelForm, resolveForm } from "@/contentScript/messenger/strict/api"; import { type Target } from "@/types/messengerTypes"; -import { templates } from "@/components/formBuilder/RjsfTemplates"; import { cloneDeep } from "lodash"; -import { useStylesheetsContextWithFormDefault } from "@/components/StylesheetsContext"; import { type FormDefinition } from "@/platform/forms/formTypes"; import { type UUID } from "@/types/stringTypes"; +import { templates } from "@/components/formBuilder/RjsfTemplates"; import ImageCropWidget from "@/components/formBuilder/ImageCropWidget"; import DescriptionField from "@/components/formBuilder/DescriptionField"; import RjsfSelectWidget from "@/components/formBuilder/RjsfSelectWidget"; @@ -40,27 +42,21 @@ export const uiWidgets = { TextareaWidget: TextAreaWidget, } as const; -export const EphemeralFormContent: React.FC<{ +export type EphemeralFormContentProps = { definition: FormDefinition; target: Target; nonce: UUID; isModal: boolean; -}> = ({ definition, target, nonce, isModal }) => { - const { - schema, - uiSchema, - cancelable, - submitCaption, - stylesheets: newStylesheets, - disableParentStyles = false, - } = definition; +}; - // Ephemeral form can never be nested, but we use this to pull in - // the (boostrap) base themes - const { stylesheets } = useStylesheetsContextWithFormDefault({ - newStylesheets, - disableParentStyles, - }); +const EphemeralFormContent: React.FC = ({ + definition, + target, + nonce, + isModal, +}) => { + const { schema, uiSchema, cancelable, submitCaption, stylesheets } = + definition; return ( @@ -98,3 +94,5 @@ export const EphemeralFormContent: React.FC<{ ); }; + +export default EphemeralFormContent; diff --git a/src/components/StylesheetsContext.ts b/src/components/StylesheetsContext.ts index 663c64500d..a9b9d98151 100644 --- a/src/components/StylesheetsContext.ts +++ b/src/components/StylesheetsContext.ts @@ -18,7 +18,6 @@ import React, { useContext } from "react"; import bootstrap from "@/vendors/bootstrapWithoutRem.css?loadAsUrl"; import bootstrapOverrides from "@/sidebar/sidebarBootstrapOverrides.scss?loadAsUrl"; -import custom from "@/bricks/renderers/customForm.css?loadAsUrl"; export type StylesheetsContextType = { stylesheets: string[] | null; @@ -81,26 +80,4 @@ export function useStylesheetsContextWithDocumentDefault({ }); } -export function useStylesheetsContextWithFormDefault({ - newStylesheets, - disableParentStyles, -}: { - newStylesheets: string[] | undefined; - disableParentStyles: boolean; -}): { - stylesheets: string[]; -} { - return useStylesheetsContextWithDefaultValues({ - newStylesheets, - defaultStylesheets: [ - bootstrap, - bootstrapOverrides, - // EphemeralFormContent.css is an artifact produced by webpack, see the entrypoints. - "/EphemeralFormContent.css", - custom, - ], - disableParentStyles, - }); -} - export default StylesheetsContext; diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs index c98a57a2e7..8a9e2d99cc 100644 --- a/src/components/isolatedComponentList.mjs +++ b/src/components/isolatedComponentList.mjs @@ -32,6 +32,7 @@ const isolatedComponentList = [ "bricks/renderers/PropertyTree", "bricks/renderers/CustomFormComponent", + "bricks/transformers/ephemeralForm/EphemeralFormContent", "components/selectionToolPopover/SelectionToolPopover", "contentScript/textSelectionMenu/SelectionMenu", ]; diff --git a/webpack.config.mjs b/webpack.config.mjs index 0ba8197088..cc8f21bd3c 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -117,7 +117,6 @@ const createConfig = (env, options) => // TODO: Move to isolatedComponentList.mjs and use "bricks/renderers/documentView/DocumentView", - "bricks/transformers/ephemeralForm/EphemeralFormContent", "contentScript/contentScript", "contentScript/loadActivationEnhancements", From 98384ca47fa2388691b3f180a77bc64cbb9352bd Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 21:08:55 +0700 Subject: [PATCH 23/33] DocumentView --- scripts/DiscardFilePlugin.mjs | 3 - .../renderers/{document.ts => document.tsx} | 22 ++++- .../renderers/documentView/DocumentView.tsx | 48 ++++------- .../documentView/DocumentViewLazy.tsx | 35 -------- .../documentView/DocumentViewProps.tsx | 5 +- src/components/StylesheetsContext.ts | 83 ------------------- src/components/isolatedComponentList.mjs | 1 + src/tsconfig.strictNullChecks.json | 1 - webpack.config.mjs | 3 - 9 files changed, 40 insertions(+), 161 deletions(-) rename src/bricks/renderers/{document.ts => document.tsx} (82%) delete mode 100644 src/bricks/renderers/documentView/DocumentViewLazy.tsx delete mode 100644 src/components/StylesheetsContext.ts diff --git a/scripts/DiscardFilePlugin.mjs b/scripts/DiscardFilePlugin.mjs index bff8ac2198..4afd567755 100644 --- a/scripts/DiscardFilePlugin.mjs +++ b/scripts/DiscardFilePlugin.mjs @@ -19,9 +19,6 @@ export default class DiscardFilePlugin { // If `delete assets[]` causes issues in the future, try replacing the content instead: // assets["DocumentView.js"] = new webpack.sources.RawSource('"Dropped"'); } - - // TODO: Use and move these to isolatedComponentList - delete assets["DocumentView.js"]; }, ); }); diff --git a/src/bricks/renderers/document.ts b/src/bricks/renderers/document.tsx similarity index 82% rename from src/bricks/renderers/document.ts rename to src/bricks/renderers/document.tsx index b7739fe732..3f2d487496 100644 --- a/src/bricks/renderers/document.ts +++ b/src/bricks/renderers/document.tsx @@ -15,8 +15,8 @@ * along with this program. If not, see . */ +import React from "react"; import { RendererABC } from "@/types/bricks/rendererTypes"; -import DocumentViewLazy from "./documentView/DocumentViewLazy"; import { validateRegistryId } from "@/types/helpers"; import { type BrickArgs, @@ -28,6 +28,8 @@ import { DOCUMENT_ELEMENT_TYPES, type DocumentElement, } from "@/components/documentBuilder/documentBuilderTypes"; +import IsolatedComponent from "@/components/IsolatedComponent"; +import { type DocumentViewProps } from "./documentView/DocumentViewProps"; export const DOCUMENT_SCHEMA: Schema = { $schema: "https://json-schema.org/draft/2019-09/schema#", @@ -107,8 +109,24 @@ export class DocumentRenderer extends RendererABC { }>, options: BrickOptions, ): Promise { + const DocumentView: React.FC< + DocumentViewProps & { disableParentStyles?: boolean } + > = ({ disableParentStyles, ...props }) => ( + + import( + /* webpackChunkName: "isolated/DocumentView" */ + "./documentView/DocumentView" + ) + } + factory={(DocumentView) => } + /> + ); + return { - Component: DocumentViewLazy, + Component: DocumentView, props: { body, stylesheets, diff --git a/src/bricks/renderers/documentView/DocumentView.tsx b/src/bricks/renderers/documentView/DocumentView.tsx index e989b8699b..d1f65c5d24 100644 --- a/src/bricks/renderers/documentView/DocumentView.tsx +++ b/src/bricks/renderers/documentView/DocumentView.tsx @@ -15,21 +15,18 @@ * along with this program. If not, see . */ +import "@/vendors/bootstrapWithoutRem.css"; +import "@/sidebar/sidebarBootstrapOverrides.scss"; import { buildDocumentBranch } from "@/components/documentBuilder/documentTree"; import React from "react"; -import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import { type DocumentViewProps } from "./DocumentViewProps"; import DocumentContext from "@/components/documentBuilder/render/DocumentContext"; import { Stylesheets } from "@/components/Stylesheets"; import { joinPathParts } from "@/utils/formUtils"; -import StylesheetsContext, { - useStylesheetsContextWithDocumentDefault, -} from "@/components/StylesheetsContext"; const DocumentView: React.FC = ({ body, - stylesheets: newStylesheets, - disableParentStyles, + stylesheets, options, meta, onAction, @@ -44,35 +41,26 @@ const DocumentView: React.FC = ({ throw new Error("meta.extensionId is required for DocumentView"); } - const { stylesheets } = useStylesheetsContextWithDocumentDefault({ - newStylesheets, - disableParentStyles, - }); - return ( // Wrap in a React context provider that passes BrickOptions down to any embedded bricks - - - - {body.map((documentElement, index) => { - const documentBranch = buildDocumentBranch(documentElement, { - staticId: joinPathParts("body", "children"), - // Root of the document, so no branches taken yet - branches: [], - }); + + {body.map((documentElement, index) => { + const documentBranch = buildDocumentBranch(documentElement, { + staticId: joinPathParts("body", "children"), + // Root of the document, so no branches taken yet + branches: [], + }); - if (documentBranch == null) { - return null; - } + if (documentBranch == null) { + return null; + } - const { Component, props } = documentBranch; - // eslint-disable-next-line react/no-array-index-key -- They have no other unique identifier - return ; - })} - - - + const { Component, props } = documentBranch; + // eslint-disable-next-line react/no-array-index-key -- They have no other unique identifier + return ; + })} + ); }; diff --git a/src/bricks/renderers/documentView/DocumentViewLazy.tsx b/src/bricks/renderers/documentView/DocumentViewLazy.tsx deleted file mode 100644 index 8ff3ef2fcf..0000000000 --- a/src/bricks/renderers/documentView/DocumentViewLazy.tsx +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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 React, { Suspense } from "react"; -import { type DocumentViewProps } from "./DocumentViewProps"; - -const DocumentView = React.lazy( - async () => - import( - /* webpackChunkName: "components-lazy" */ - "./DocumentView" - ), -); - -const DocumentViewLazy: React.FC = (props) => ( - - - -); - -export default DocumentViewLazy; diff --git a/src/bricks/renderers/documentView/DocumentViewProps.tsx b/src/bricks/renderers/documentView/DocumentViewProps.tsx index 7ebc9c1dd5..65fca3255c 100644 --- a/src/bricks/renderers/documentView/DocumentViewProps.tsx +++ b/src/bricks/renderers/documentView/DocumentViewProps.tsx @@ -33,10 +33,7 @@ export type DocumentViewProps = { * Remote stylesheets (URLs) to include in the document. */ stylesheets?: string[]; - /** - * Whether to disable the base (bootstrap) styles, plus any inherited styles, on the document (and children). - */ - disableParentStyles?: boolean; + options: BrickOptions; meta: { runId: UUID; diff --git a/src/components/StylesheetsContext.ts b/src/components/StylesheetsContext.ts deleted file mode 100644 index a9b9d98151..0000000000 --- a/src/components/StylesheetsContext.ts +++ /dev/null @@ -1,83 +0,0 @@ -/* - * 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 React, { useContext } from "react"; -import bootstrap from "@/vendors/bootstrapWithoutRem.css?loadAsUrl"; -import bootstrapOverrides from "@/sidebar/sidebarBootstrapOverrides.scss?loadAsUrl"; - -export type StylesheetsContextType = { - stylesheets: string[] | null; -}; - -const StylesheetsContext = React.createContext({ - stylesheets: null, -}); - -function useStylesheetsContextWithDefaultValues({ - newStylesheets, - defaultStylesheets, - disableParentStyles, -}: { - newStylesheets: string[] | undefined; - defaultStylesheets: string[]; - disableParentStyles: boolean; -}): { - stylesheets: string[]; -} { - const { stylesheets: inheritedStylesheets } = useContext(StylesheetsContext); - - const stylesheets: string[] = []; - - if (!disableParentStyles) { - if (inheritedStylesheets == null) { - stylesheets.push(...defaultStylesheets); - } else { - stylesheets.push(...inheritedStylesheets); - } - } - - if (newStylesheets != null) { - stylesheets.push(...newStylesheets); - } - - return { stylesheets }; -} - -export function useStylesheetsContextWithDocumentDefault({ - newStylesheets, - disableParentStyles, -}: { - newStylesheets: string[] | undefined; - disableParentStyles: boolean; -}): { - stylesheets: string[]; -} { - return useStylesheetsContextWithDefaultValues({ - newStylesheets, - defaultStylesheets: [ - bootstrap, - bootstrapOverrides, - // DocumentView.css is an artifact produced by webpack, see the DocumentView entrypoint included in - // `webpack.config.mjs`. We build styles needed to render documents separately from the rest of the sidebar - // in order to isolate the rendered document from the custom Bootstrap theme included in the Sidebar app - "/DocumentView.css", - ], - disableParentStyles, - }); -} - -export default StylesheetsContext; diff --git a/src/components/isolatedComponentList.mjs b/src/components/isolatedComponentList.mjs index 8a9e2d99cc..7a766eabf6 100644 --- a/src/components/isolatedComponentList.mjs +++ b/src/components/isolatedComponentList.mjs @@ -32,6 +32,7 @@ const isolatedComponentList = [ "bricks/renderers/PropertyTree", "bricks/renderers/CustomFormComponent", + "bricks/renderers/documentView/DocumentView", "bricks/transformers/ephemeralForm/EphemeralFormContent", "components/selectionToolPopover/SelectionToolPopover", "contentScript/textSelectionMenu/SelectionMenu", diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 1a62f1c572..da4276e083 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -208,7 +208,6 @@ "./components/StopPropagation.tsx", "./components/Stylesheets.test.tsx", "./components/Stylesheets.tsx", - "./components/StylesheetsContext.ts", "./components/TooltipIconButton.tsx", "./components/UnstyledButton.tsx", "./components/addBlockModal/TagList.tsx", diff --git a/webpack.config.mjs b/webpack.config.mjs index cc8f21bd3c..b38eaf9109 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -115,9 +115,6 @@ const createConfig = (env, options) => [ "background/background", - // TODO: Move to isolatedComponentList.mjs and use - "bricks/renderers/documentView/DocumentView", - "contentScript/contentScript", "contentScript/loadActivationEnhancements", "contentScript/browserActionInstantHandler", From e29f97408f9c8fb4dc4245dbf35077848b7040ab Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 22:45:57 +0700 Subject: [PATCH 24/33] Bad merge --- src/contentScript/pageEditor/selectElement.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contentScript/pageEditor/selectElement.test.ts b/src/contentScript/pageEditor/selectElement.test.ts index 2d62398654..96fe5e90d7 100644 --- a/src/contentScript/pageEditor/selectElement.test.ts +++ b/src/contentScript/pageEditor/selectElement.test.ts @@ -19,7 +19,7 @@ import selectElement from "./selectElement"; import showSelectionToolPopover from "@/components/selectionToolPopover/showSelectionToolPopover"; // Mock because the React vs. JSDOM event handling and dom manipulation isn't playing nicely together -jest.mock("@/components/selectionToolPopover/SelectionToolPopover"); +jest.mock("@/components/selectionToolPopover/showSelectionToolPopover"); const showSelectionToolPopoverMock = jest.mocked(showSelectionToolPopover); From 98b0f8efdefd8921745fc636ba0b04b51618ad24 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 8 Apr 2024 23:07:52 +0700 Subject: [PATCH 25/33] Lint --- src/bricks/renderers/customForm.tsx | 5 ++- src/bricks/renderers/document.tsx | 5 +-- .../ephemeralForm/EphemeralForm.tsx | 32 +++++++++---------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/bricks/renderers/customForm.tsx b/src/bricks/renderers/customForm.tsx index d0444b88aa..b514d769dd 100644 --- a/src/bricks/renderers/customForm.tsx +++ b/src/bricks/renderers/customForm.tsx @@ -301,8 +301,8 @@ export class CustomFormRenderer extends RendererABC { }); const CustomFormComponent: React.FunctionComponent< - CustomFormComponentProps & { disableParentStyles?: boolean } - > = ({ disableParentStyles, ...props }) => ( + CustomFormComponentProps + > = (props) => ( , options: BrickOptions, ): Promise { - const DocumentView: React.FC< - DocumentViewProps & { disableParentStyles?: boolean } - > = ({ disableParentStyles, ...props }) => ( + const DocumentView: React.FC = (props) => ( (
{children}
); +const EphemeralFormContent: React.FunctionComponent< + EphemeralFormContentProps +> = (props) => ( + + import( + /* webpackChunkName: "isolated/EphemeralFormContent" */ + "./EphemeralFormContent" + ) + } + factory={(EphemeralFormContent) => } + /> +); + /** * @see FormTransformer */ @@ -98,22 +114,6 @@ const EphemeralForm: React.FC = () => { ); } - const EphemeralFormContent: React.FunctionComponent< - EphemeralFormContentProps - > = (props) => ( - - import( - /* webpackChunkName: "isolated/EphemeralFormContent" */ - "./EphemeralFormContent" - ) - } - factory={(EphemeralFormContent) => } - /> - ); - return ( From 82b3dd41898664a57b83f83f9a8129cae926f5f6 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Tue, 9 Apr 2024 01:01:04 +0700 Subject: [PATCH 26/33] Lint --- package-lock.json | 8 +++---- package.json | 2 +- src/components/IsolatedComponent.tsx | 24 +++++++------------ .../showSelectionToolPopover.tsx | 2 +- 4 files changed, 14 insertions(+), 22 deletions(-) diff --git a/package-lock.json b/package-lock.json index 608d23ae09..49accb394c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,7 +29,7 @@ "@uipath/robot": "1.3.1", "@vespaiach/axios-fetch-adapter": "^0.3.1", "@xobotyi/scrollbar-width": "^1.9.5", - "abort-utils": "^1.1.0", + "abort-utils": "^1.2.0", "ace-builds": "^1.32.9", "autocompleter": "^9.1.2", "axios": "^0.27.2", @@ -10273,9 +10273,9 @@ } }, "node_modules/abort-utils": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/abort-utils/-/abort-utils-1.1.0.tgz", - "integrity": "sha512-c/RkrNs9HS8F+cnX+yNidnbVI4LPTM56fs50CFuMX2V/KNkmjYxBhMeQyvy3zlhVZcZsc7PDwesbju2XCWmmKQ==", + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/abort-utils/-/abort-utils-1.2.0.tgz", + "integrity": "sha512-C++yavghYIAoAtd7NDHqVEgNLoZVNIxCNZQZrQd8mbHY173NpT/qOJmGU9Z0565GiCA7kqXWMjaVchIP4F0WMg==", "engines": { "node": ">=18" }, diff --git a/package.json b/package.json index 2c9e20e5b7..7c567b0ae2 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "@uipath/robot": "1.3.1", "@vespaiach/axios-fetch-adapter": "^0.3.1", "@xobotyi/scrollbar-width": "^1.9.5", - "abort-utils": "^1.1.0", + "abort-utils": "^1.2.0", "ace-builds": "^1.32.9", "autocompleter": "^9.1.2", "axios": "^0.27.2", diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index 50825921c5..af10027224 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -23,6 +23,7 @@ import React, { Suspense } from "react"; import { Stylesheets } from "@/components/Stylesheets"; import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import isolatedComponentList from "./isolatedComponentList"; +import { signalFromPromise } from "abort-utils"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; @@ -32,13 +33,10 @@ type LazyFactory = () => Promise<{ // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. // Until this is resolved https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 -async function discardStylesheetsWhilePending( - lazyFactory: LazyFactory, -) { +async function discardNewStylesheets(signal: AbortSignal) { const baseUrl = chrome.runtime.getURL(""); const observer = new MutationObserver((mutations) => { - // Use for of for (const mutation of mutations) { for (const node of mutation.addedNodes) { if (node instanceof HTMLLinkElement && node.href.startsWith(baseUrl)) { @@ -54,15 +52,9 @@ async function discardStylesheetsWhilePending( childList: true, }); - // Call and discard. React.lazy() will call it again and use the result or the error. - // This is fine because import() does not re-fetch/re-run the module. - try { - await lazyFactory(); - } catch { - // React.lazy() will take care of it - } finally { + signal.addEventListener("abort", () => { observer.disconnect(); - } + }); } type Props = { @@ -119,12 +111,12 @@ export default function IsolatedComponent({ ); } - const stylesheetUrl = noStyle ? null : chrome.runtime.getURL(`${name}.css`); - - // `discard` one must be called before `React.lazy` - void discardStylesheetsWhilePending(lazy); + // `discard` must be called before `React.lazy` + void discardNewStylesheets(signalFromPromise(lazy())); const LazyComponent = React.lazy(lazy); + const stylesheetUrl = noStyle ? null : chrome.runtime.getURL(`${name}.css`); + return ( // `pb-name` is used to visually identify it in the dev tools diff --git a/src/components/selectionToolPopover/showSelectionToolPopover.tsx b/src/components/selectionToolPopover/showSelectionToolPopover.tsx index 1b0c180ff4..f3cfedbfca 100644 --- a/src/components/selectionToolPopover/showSelectionToolPopover.tsx +++ b/src/components/selectionToolPopover/showSelectionToolPopover.tsx @@ -36,7 +36,7 @@ export default function showSelectionToolPopover({ ) } factory={(SelectionToolPopover) => } - >, + />, rootElement, ); } From 1334a917b5e471f40592da53e42d98f267fecc75 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 10 Apr 2024 17:49:41 +0700 Subject: [PATCH 27/33] Sort jest config --- jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index 7e8fed1de0..05fe397960 100644 --- a/jest.config.js +++ b/jest.config.js @@ -98,6 +98,9 @@ const config = { "!**/vendor/**", ], moduleNameMapper: { + "^@contrib/([^?]+)": "/contrib/$1", + "^@schemas/([^?]+)": "/schemas/$1", + "\\.s?css$": "identity-obj-proxy", "\\.(gif|svg|png)$": "/src/__mocks__/stringMock.js", @@ -105,9 +108,6 @@ const config = { "\\?loadAsText$": "/src/__mocks__/stringMock.js", "\\?loadAsComponent$": "/src/__mocks__/stringMock.js", - "^@contrib/(.*?)(\\?loadAsText)?$": "/contrib/$1", - "^@schemas/(.*)": "/schemas/$1", - // Auto-mocks. See documentation in ./src/__mocks__/readme.md "^@/(.*)$": ["/src/__mocks__/@/$1", "/src/$1"], }, From 36956810f26b1ca14a6d015ee8256e498a280040 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 10 Apr 2024 22:41:50 +0700 Subject: [PATCH 28/33] Ensure stylesheets are removed in IsolatedComponent --- src/components/IsolatedComponent.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index af10027224..cd8e9dd38e 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -23,7 +23,6 @@ import React, { Suspense } from "react"; import { Stylesheets } from "@/components/Stylesheets"; import EmotionShadowRoot from "@/components/EmotionShadowRoot"; import isolatedComponentList from "./isolatedComponentList"; -import { signalFromPromise } from "abort-utils"; const MODE = process.env.SHADOW_DOM as "open" | "closed"; @@ -33,7 +32,9 @@ type LazyFactory = () => Promise<{ // Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. // Until this is resolved https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 -async function discardNewStylesheets(signal: AbortSignal) { +async function discardStylesheetsWhilePending( + lazyFactory: LazyFactory, +) { const baseUrl = chrome.runtime.getURL(""); const observer = new MutationObserver((mutations) => { @@ -52,9 +53,16 @@ async function discardNewStylesheets(signal: AbortSignal) { childList: true, }); - signal.addEventListener("abort", () => { + // Call and discard. React.lazy() will call it again and use the result or the error. + // This is fine because import() does not re-fetch/re-run the module. + try { + // The function must be first called *after* the observer is set up. + await lazyFactory(); + } catch { + // React.lazy() will take care of it + } finally { observer.disconnect(); - }); + } } type Props = { @@ -112,7 +120,7 @@ export default function IsolatedComponent({ } // `discard` must be called before `React.lazy` - void discardNewStylesheets(signalFromPromise(lazy())); + void discardStylesheetsWhilePending(lazy); const LazyComponent = React.lazy(lazy); const stylesheetUrl = noStyle ? null : chrome.runtime.getURL(`${name}.css`); From 9d86817704cb9c4f01ca58efe25bae73f8af7e8c Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 10 Apr 2024 22:55:51 +0700 Subject: [PATCH 29/33] Comments --- src/components/IsolatedComponent.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index cd8e9dd38e..c31d02a87e 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -30,8 +30,12 @@ type LazyFactory = () => Promise<{ default: React.ComponentType; }>; -// Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. -// Until this is resolved https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 +/** + * Drop the stylesheet injected by `mini-css-extract-plugin` into the main document. + * + * @warning The `lazyFactory` function never not be called outside `discardStylesheetsWhilePending` + * because this helper must catch the stylesheets injected when the factory is first called. + */ async function discardStylesheetsWhilePending( lazyFactory: LazyFactory, ) { @@ -119,7 +123,8 @@ export default function IsolatedComponent({ ); } - // `discard` must be called before `React.lazy` + // `discard` must be called before `React.lazy`. + // `discardStylesheetsWhilePending` is needed until this is resolved https://github.com/webpack-contrib/mini-css-extract-plugin/issues/1092#issuecomment-2037540032 void discardStylesheetsWhilePending(lazy); const LazyComponent = React.lazy(lazy); From c7aaa29eee8c10079c0fc6b870bbab658dff2c4d Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 10 Apr 2024 23:37:58 +0700 Subject: [PATCH 30/33] Open Shadow DOM To avoid this regression: https://github.com/pixiebrix/pixiebrix-extension/pull/8211 --- webpack.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webpack.config.mjs b/webpack.config.mjs index b38eaf9109..8757436d0d 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -252,7 +252,7 @@ const createConfig = (env, options) => DEV_EVENT_TELEMETRY: false, SANDBOX_LOGGING: false, IS_BETA: process.env.PUBLIC_NAME === "-beta", - SHADOW_DOM: "closed", + SHADOW_DOM: "open", // If not found, "undefined" will cause the build to fail SERVICE_URL: undefined, From ee2aa0afbfba1b567654b00c1971ffd0664e833c Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Wed, 10 Apr 2024 23:56:49 +0700 Subject: [PATCH 31/33] Test shadow DOM --- src/bricks/renderers/customForm.test.tsx | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/bricks/renderers/customForm.test.tsx b/src/bricks/renderers/customForm.test.tsx index d882062302..e2384e0457 100644 --- a/src/bricks/renderers/customForm.test.tsx +++ b/src/bricks/renderers/customForm.test.tsx @@ -16,7 +16,8 @@ */ import React from "react"; -import { render, screen } from "@testing-library/react"; +import { render } from "@testing-library/react"; +import { screen } from "shadow-dom-testing-library"; import ImageCropWidget from "@/components/formBuilder/ImageCropWidget"; import DescriptionField from "@/components/formBuilder/DescriptionField"; import JsonSchemaForm from "@rjsf/bootstrap-4"; @@ -228,9 +229,10 @@ describe("CustomFormRenderer", () => { ); render(); + await waitForEffect(); expect(screen.queryByText("Submit")).not.toBeInTheDocument(); - expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(screen.getByShadowRole("textbox")).toBeInTheDocument(); }); test("Supports postSubmitAction reset", async () => { @@ -261,17 +263,19 @@ describe("CustomFormRenderer", () => { render(); - const textBox = screen.getByRole("textbox"); + await waitForEffect(); + + const textBox = screen.getByShadowRole("textbox"); await userEvent.type(textBox, "Some text"); expect(textBox).toHaveValue("Some text"); - await userEvent.click(screen.getByRole("button", { name: "Submit" })); + await userEvent.click(screen.getByShadowRole("button", { name: "Submit" })); expect(runPipelineMock).toHaveBeenCalledOnce(); expect(dataStoreSetSpy).not.toHaveBeenCalled(); // Need to get new textbox reference, because the old one is removed when the key changes - expect(screen.getByRole("textbox")).toHaveValue(""); + expect(screen.getByShadowRole("textbox")).toHaveValue(""); }); test.each([undefined, "save"])( @@ -304,10 +308,14 @@ describe("CustomFormRenderer", () => { render(); + await waitForEffect(); + const value = "Some text"; - const textBox = screen.getByRole("textbox"); + const textBox = screen.getByShadowRole("textbox"); await userEvent.type(textBox, value); - await userEvent.click(screen.getByRole("button", { name: "Submit" })); + await userEvent.click( + screen.getByShadowRole("button", { name: "Submit" }), + ); expect(runPipelineMock).toHaveBeenCalledOnce(); expect(dataStoreSetSpy).toHaveBeenCalledExactlyOnceWith("test", { @@ -315,7 +323,7 @@ describe("CustomFormRenderer", () => { }); // Need to get new textbox reference, because the old one is removed if/when the key changes - expect(screen.getByRole("textbox")).toHaveValue(value); + expect(screen.getByShadowRole("textbox")).toHaveValue(value); }, ); From 9db1a1d148111de006e71313f2e44a8f1b9e36b4 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Fri, 12 Apr 2024 00:15:13 +0700 Subject: [PATCH 32/33] Fill the frame (h-100) --- src/bricks/renderers/document.tsx | 5 +++++ src/components/EmotionShadowRoot.ts | 6 +++--- src/components/IsolatedComponent.tsx | 7 ++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/bricks/renderers/document.tsx b/src/bricks/renderers/document.tsx index 527ae4884f..bee5d75207 100644 --- a/src/bricks/renderers/document.tsx +++ b/src/bricks/renderers/document.tsx @@ -120,6 +120,11 @@ export class DocumentRenderer extends RendererABC { ) } factory={(DocumentView) => } + // It must fill the frame even if `noStyle` is set, so set it as a style prop + // TODO: The parent node should instead make sure that the children fill + // the sidebar vertically (via a simple `.d-flex`), but this this requires + // verifying that other components aren't broken by this. + style={{ height: "100%" }} /> ); diff --git a/src/components/EmotionShadowRoot.ts b/src/components/EmotionShadowRoot.ts index 8d0fad6a7a..697962c619 100644 --- a/src/components/EmotionShadowRoot.ts +++ b/src/components/EmotionShadowRoot.ts @@ -20,7 +20,7 @@ Also strictNullChecks config mismatch */ */ // eslint-disable-next-line no-restricted-imports -- All roads lead here -import EmotionShadowRoot from "react-shadow/emotion"; +import ShadowRoot from "react-shadow/emotion"; import { type CSSProperties } from "react"; /** @@ -28,7 +28,7 @@ import { type CSSProperties } from "react"; * the host website. To support react-select and any future potential emotion * components we used the emotion variant of the react-shadow library. */ -const ShadowRoot = EmotionShadowRoot.div!; +const EmotionShadowRoot = ShadowRoot.div!; // TODO: Use EmotionShadowRoot["pixiebrix-widget"] to avoid any CSS conflicts. Requires snapshot/test updates export const styleReset: CSSProperties = { @@ -36,4 +36,4 @@ export const styleReset: CSSProperties = { font: "16px / 1.5 sans-serif", }; -export default ShadowRoot; +export default EmotionShadowRoot; diff --git a/src/components/IsolatedComponent.tsx b/src/components/IsolatedComponent.tsx index c7a073c955..e1fc944463 100644 --- a/src/components/IsolatedComponent.tsx +++ b/src/components/IsolatedComponent.tsx @@ -69,7 +69,10 @@ async function discardStylesheetsWhilePending( } } -type Props = { +type Props = React.DetailedHTMLProps< + React.HTMLAttributes, + HTMLDivElement +> & { /** * It must match the `import()`ed component's filename */ @@ -100,6 +103,8 @@ type Props = { /** * Isolate component loaded via React.lazy() in a shadow DOM, including its styles. * + * Additional props will be passed to the Shadow DOM root element. + * * @example * render( * Date: Thu, 11 Apr 2024 12:57:02 -0500 Subject: [PATCH 33/33] fixes failing tests --- src/bricks/renderers/customForm.test.tsx | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/bricks/renderers/customForm.test.tsx b/src/bricks/renderers/customForm.test.tsx index e2384e0457..07db46a69c 100644 --- a/src/bricks/renderers/customForm.test.tsx +++ b/src/bricks/renderers/customForm.test.tsx @@ -27,7 +27,6 @@ import { normalizeIncomingFormData, normalizeOutgoingFormData, } from "./customForm"; -import { waitForEffect } from "@/testUtils/testHelpers"; import userEvent from "@testing-library/user-event"; import { dataStore } from "@/background/messenger/strict/api"; @@ -39,6 +38,16 @@ import { toExpression } from "@/utils/expressionUtils"; const dataStoreGetMock = jest.mocked(dataStore.get); const dataStoreSetSpy = jest.spyOn(dataStore, "set"); +// I couldn't get shadow-dom-testing-library working +jest.mock("react-shadow/emotion", () => ({ + __esModule: true, + default: { + div(props: any) { + return
; + }, + }, +})); + describe("form data normalization", () => { const normalizationTestCases = [ { @@ -187,7 +196,7 @@ describe("form data normalization", () => { />, ); - await waitForEffect(); + await expect(screen.findByRole("button")).resolves.toBeInTheDocument(); // Make sure the form renders the data without errors expect(asFragment()).toMatchSnapshot(); @@ -229,10 +238,9 @@ describe("CustomFormRenderer", () => { ); render(); - await waitForEffect(); expect(screen.queryByText("Submit")).not.toBeInTheDocument(); - expect(screen.getByShadowRole("textbox")).toBeInTheDocument(); + await expect(screen.findByRole("textbox")).resolves.toBeInTheDocument(); }); test("Supports postSubmitAction reset", async () => { @@ -263,19 +271,17 @@ describe("CustomFormRenderer", () => { render(); - await waitForEffect(); - - const textBox = screen.getByShadowRole("textbox"); + const textBox = await screen.findByRole("textbox"); await userEvent.type(textBox, "Some text"); expect(textBox).toHaveValue("Some text"); - await userEvent.click(screen.getByShadowRole("button", { name: "Submit" })); + await userEvent.click(screen.getByRole("button", { name: "Submit" })); expect(runPipelineMock).toHaveBeenCalledOnce(); expect(dataStoreSetSpy).not.toHaveBeenCalled(); // Need to get new textbox reference, because the old one is removed when the key changes - expect(screen.getByShadowRole("textbox")).toHaveValue(""); + expect(screen.getByRole("textbox")).toHaveValue(""); }); test.each([undefined, "save"])( @@ -308,14 +314,12 @@ describe("CustomFormRenderer", () => { render(); - await waitForEffect(); - const value = "Some text"; - const textBox = screen.getByShadowRole("textbox"); + const textBox = await screen.findByRole("textbox"); + await userEvent.type(textBox, value); - await userEvent.click( - screen.getByShadowRole("button", { name: "Submit" }), - ); + + await userEvent.click(screen.getByRole("button", { name: "Submit" })); expect(runPipelineMock).toHaveBeenCalledOnce(); expect(dataStoreSetSpy).toHaveBeenCalledExactlyOnceWith("test", { @@ -323,7 +327,7 @@ describe("CustomFormRenderer", () => { }); // Need to get new textbox reference, because the old one is removed if/when the key changes - expect(screen.getByShadowRole("textbox")).toHaveValue(value); + expect(screen.getByRole("textbox")).toHaveValue(value); }, );