Fixes brick position reset and simplifies form reinitialization

end-to-end-tests/tests/pageEditor/brickActions.spec.ts-snapshots/brick-actions-panel-behavior/brick-added.diff

@@ -2,7 +2,7 @@ Index: brick-added
 ===================================================================
 --- brick-added
 +++ brick-added
-@@ -6,78 +6,85 @@
+@@ -6,78 +6,84 @@
        containerSelector: span:has(> span:contains('Review in codespace'))
        isAvailable:
          allFrames: true
@@ -47,7 +47,6 @@ Index: brick-added
 +            key: !nunjucks ''
 +          id: '@pixies/util/set-mod-variable'
 +          outputKey: output
-+          root: null
 +          rootMode: document
 +        - config:
              value: !nunjucks ''

end-to-end-tests/tests/pageEditor/brickActions.spec.ts-snapshots/brick-actions-panel-behavior/brick-removed.diff

@@ -2,7 +2,7 @@ Index: brick-removed
 ===================================================================
 --- brick-removed
 +++ brick-removed
-@@ -6,85 +6,78 @@
+@@ -6,84 +6,78 @@
        containerSelector: span:has(> span:contains('Review in codespace'))
        isAvailable:
          allFrames: true
@@ -47,7 +47,6 @@ Index: brick-removed
 -            key: !nunjucks ''
 -          id: '@pixies/util/set-mod-variable'
 -          outputKey: output
--          root: null
 -          rootMode: document
 -        - config:
              value: !nunjucks ''

src/pageEditor/panes/ModComponentEditorPane.test.tsx

@@ -16,7 +16,7 @@
  */
 
 import React from "react";
-import { render, screen, within } from "@/pageEditor/testHelpers";
+import { render, screen, within, userEvent } from "@/pageEditor/testHelpers";
 import ModComponentEditorPane from "./ModComponentEditorPane";
 import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
 import { selectActiveModComponentFormState } from "@/pageEditor/store/editor/editorSelectors";
@@ -26,7 +26,6 @@ import { echoBrick, teapotBrick } from "@/runtime/pipelineTests/testHelpers";
 import { defaultBrickConfig } from "@/bricks/util";
 import { waitForEffect } from "@/testUtils/testHelpers";
 import registerDefaultWidgets from "@/components/fields/schemaFields/widgets/registerDefaultWidgets";
-import userEvent from "@testing-library/user-event";
 import { JQTransformer } from "@/bricks/transformers/jq";
 import { AlertEffect } from "@/bricks/effects/alert";
 import ForEach from "@/bricks/transformers/controlFlow/ForEach";
@@ -430,6 +429,7 @@ async function renderEditorPaneWithBasicFormState() {
   const modComponentFormState = getFormStateWithSubPipelines();
   const activeNodeId =
     modComponentFormState.modComponent.brickPipeline[0]!.instanceId;
+
   const utils = render(
     <div>
       <ModComponentEditorPane />
@@ -473,7 +473,7 @@ describe("can remove a node", () => {
     await renderEditorPaneWithBasicFormState();
 
     // Nodes are: Foundation, Echo, ForEach: [Echo]
-    // Select the second Echo block
+    // Select the second Echo brick
     await immediateUserEvent.click(screen.getAllByText(/echo brick/i)[1]!);
 
     // Click the remove button
@@ -655,6 +655,7 @@ describe("can copy and paste a node", () => {
     // There should be 5 paste buttons
     const pasteButtons = screen.getAllByTestId(/-paste-brick/i);
     expect(pasteButtons).toHaveLength(5);
+
     // Click the last one
     await immediateUserEvent.click(pasteButtons[4]!);
 
@@ -671,7 +672,6 @@ describe("can copy and paste a node", () => {
 
 describe("validation", () => {
   function expectEditorError(container: HTMLElement, errorMessage: string) {
-    // eslint-disable-next-line testing-library/no-node-access -- TODO: use a better selector
     const errorBadge = container.querySelector(
       '.active[data-testid="editor-node"] span.badge',
     );
@@ -764,7 +764,7 @@ describe("validation", () => {
 
     // Ensure 2 nodes have error badges
     expect(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- TODO: use a better selector
+      // eslint-disable-next-line testing-library/no-container -- TODO: use a better selector
       container.querySelectorAll('[data-testid="editor-node"] span.badge'),
     ).toHaveLength(2);
 
@@ -773,7 +773,7 @@ describe("validation", () => {
     store.dispatch(editorActions.setActiveModComponentId(modComponent2.uuid));
 
     // Ensure no error is displayed
-    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- TODO: use a better selector
+    // eslint-disable-next-line testing-library/no-container -- TODO: use a better selector
     const errorBadgesOfAnotherModComponent = container.querySelectorAll(
       '[data-testid="editor-node"] span.badge',
     );
@@ -787,7 +787,7 @@ describe("validation", () => {
 
     // Should show 2 error in the Node Layout
     expect(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- TODO: use a better selector
+      // eslint-disable-next-line testing-library/no-container -- TODO: use a better selector
       container.querySelectorAll('[data-testid="editor-node"] span.badge'),
     ).toHaveLength(2);
 
@@ -992,9 +992,8 @@ describe("brick validation in Add Brick Modal UI", () => {
       await tickAsyncEffects();
 
       // Check for the alert on hover
-      const firstResult =
-        // eslint-disable-next-line testing-library/no-node-access -- TODO: use a better selector
-        screen.getAllByRole("button", { name: /add/i })[0]!.parentElement!;
+      const firstResult = screen.getAllByRole("button", { name: /add/i })[0]!
+        .parentElement!;
       await immediateUserEvent.hover(firstResult);
       expect(firstResult).toHaveTextContent("is not allowed in this pipeline");
     },
@@ -1037,7 +1036,6 @@ describe("brick validation in Add Brick Modal UI", () => {
 
     // Assert that no UiPath bricks are available
     for (const button of addButtons) {
-      // eslint-disable-next-line testing-library/no-node-access -- TODO: use a better selector
       const brick = button.parentElement;
       expect(brick).not.toHaveTextContent("uipath");
     }

src/pageEditor/panes/ModComponentEditorPane.tsx

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import React, { useEffect } from "react";
+import React, { useEffect, useMemo, useRef } from "react";
 import {
   actions,
   actions as editorActions,
@@ -36,6 +36,7 @@ import {
 import IntegrationsSliceModIntegrationsContextAdapter from "@/integrations/store/IntegrationsSliceModIntegrationsContextAdapter";
 import { assertNotNullish } from "@/utils/nullishUtils";
 import useRegisterDraftModInstanceOnAllFrames from "@/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames";
+import { usePreviousValue } from "@/hooks/usePreviousValue";
 
 // CHANGE_DETECT_DELAY_MILLIS should be low enough so that sidebar gets updated in a reasonable amount of time, but
 // high enough that there isn't an entry lag in the page editor
@@ -87,26 +88,45 @@ const EditorPaneContent: React.VoidFunctionComponent<{
   );
 };
 
-const ModComponentEditorPane: React.VFC = () => {
-  // Inject the draft mod instance into the page while editing
-  useRegisterDraftModInstanceOnAllFrames();
-
+/**
+ * Returns the active mod component form state. Responds to updates in the editor state for use with triggering rerenders.
+ */
+function useInitialValues(): ModComponentFormState {
+  const editorUpdateKey = useSelector(selectEditorUpdateKey);
   const activeModComponentFormState = useSelector(
     selectActiveModComponentFormState,
   );
+
   assertNotNullish(
     activeModComponentFormState,
     "ModComponentEditorPane requires activeModComponentFormState",
   );
-  const editorUpdateKey = useSelector(selectEditorUpdateKey);
-  // Key to force reload of component when user selects a different mod component from the sidebar
+
+  // Key to force reinitialization of formik when user selects a different mod component from the sidebar
   const key = `${activeModComponentFormState.uuid}-${activeModComponentFormState.installed}-${editorUpdateKey}`;
+  const prevKey = usePreviousValue(key);
+  const activeModComponentFormStateRef = useRef(activeModComponentFormState);
+
+  return useMemo(() => {
+    if (key === prevKey) {
+      return activeModComponentFormStateRef.current;
+    }
+
+    activeModComponentFormStateRef.current = activeModComponentFormState;
+    return activeModComponentFormState;
+  }, [key, prevKey, activeModComponentFormState]);
+}
+
+const ModComponentEditorPane: React.VFC = () => {
+  // Inject the draft mod instance into the page while editing
+  useRegisterDraftModInstanceOnAllFrames();
+  const initialValues = useInitialValues();
 
   return (
-    <ErrorBoundary key={key}>
+    <ErrorBoundary>
       <Formik
-        key={key}
-        initialValues={activeModComponentFormState}
+        enableReinitialize
+        initialValues={initialValues}
         onSubmit={() => {
           console.error(
             "Formik's submit should not be called to save a mod component.",

src/pageEditor/store/editor/editorSelectors/editorPipelineSelectors.ts

@@ -50,10 +50,7 @@ export const selectActiveNodeId = createSelector(
 export const selectActiveNodeInfo = createSelector(
   selectActiveBrickPipelineUIState,
   selectActiveNodeId,
-  (
-    uiState: Nullishable<BrickPipelineUIState>,
-    activeNodeId: Nullishable<UUID>,
-  ) => {
+  (uiState, activeNodeId) => {
     assertNotNullish(
       uiState,
       `uiState is ${typeof uiState === "object" ? "null" : "undefined"}`,

src/pageEditor/tabs/editTab/editorNodeLayout/usePipelineNodes.tsx

@@ -372,15 +372,15 @@ const usePipelineNodes = (): {
       }));
     };
 
-    const showAddBlock = isApiAtLeastV2 && (index < lastIndex || showAppend);
+    const showAddBrick = isApiAtLeastV2 && (index < lastIndex || showAppend);
     const showBiggerActions = index === lastIndex && isRootPipeline;
-    const showAddMessage = showAddBlock && showBiggerActions;
+    const showAddMessage = showAddBrick && showBiggerActions;
 
     const brickNodeActions: NodeAction[] = [];
     const nodeId = instanceId;
 
     // TODO: Refactoring - remove code duplication in the node actions here
-    if (showAddBlock) {
+    if (showAddBrick) {
       brickNodeActions.push({
         name: `${nodeId}-add-brick`,
         icon: faPlusCircle,
@@ -415,7 +415,7 @@ const usePipelineNodes = (): {
     };
 
     if (block) {
-      assertNotNullish(nodeId, "nodeId is required to get block annotations");
+      assertNotNullish(nodeId, "nodeId is required to get brick annotations");
       // Handle race condition on pipelineMap updates
       // eslint-disable-next-line security/detect-object-injection -- relying on nodeId being a UUID
       const blockPath = maybePipelineMap?.[nodeId]?.path;

src/pageEditor/tabs/effect/BrickConfiguration.tsx

@@ -51,7 +51,9 @@ import { useDispatch } from "react-redux";
 import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
 
 /**
- * Handles possible NPE from `config.value` being undefined.
+ * Handles possible NPE from the brick configuration not being migrated.
+ * Ensures that the brickPipelineUIState is valid
+ *
  * This originally happened during the extension -> modComponent form state migration.
  * See https://github.com/pixiebrix/pixiebrix-extension/issues/8781
  */
@@ -60,7 +62,12 @@ function useRecoverFormStateIntegrityError(name: string) {
   const [config] = useField<BrickConfig | undefined>(name);
   const dispatch = useDispatch();
 
-  if (!config.value) {
+  /**
+   * If the name doesn't include "modComponent", the brickPipelineUIstate has not been migrated
+   * and is invalid. Previously, we checked for the existence of config.value, but config.value can be undefined
+   * (see usage in BrickConfiguration). Remounting the form instead of reinitializing hid that logic error.
+   */
+  if (!name.includes("modComponent")) {
     dispatch(
       editorActions.setModComponentFormState({
         modComponentFormState: context.values,