Drop currentModId as a concept

applications/browser-extension/end-to-end-tests/tests/pageEditor/saveMod.spec.ts

@@ -52,7 +52,6 @@ test("can save a new trigger mod", async ({
 
 test("#9349: can save new mod with multiple components", async ({
   page,
-  extensionId,
   newPageEditorPage,
 }) => {
   await page.goto("/");

applications/browser-extension/src/pageEditor/hooks/useEnsureFormStates.ts

@@ -18,7 +18,7 @@
 import { actions } from "@/pageEditor/store/editor/editorSlice";
 import { modComponentToFormState } from "@/pageEditor/starterBricks/adapter";
 import {
-  selectCurrentModId,
+  selectActiveModId,
   selectGetUntouchedActivatedModComponentsForMod,
 } from "@/pageEditor/store/editor/editorSelectors";
 import { useDispatch, useSelector } from "react-redux";
@@ -30,13 +30,13 @@ import { useEffect } from "react";
  */
 function useEnsureFormStates(): void {
   const dispatch = useDispatch();
-  const currentModId = useSelector(selectCurrentModId);
+  const activeModId = useSelector(selectActiveModId);
   const getUntouchedActivatedModComponentsForMod = useSelector(
     selectGetUntouchedActivatedModComponentsForMod,
   );
 
-  const untouchedModComponents = currentModId
-    ? getUntouchedActivatedModComponentsForMod(currentModId)
+  const untouchedModComponents = activeModId
+    ? getUntouchedActivatedModComponentsForMod(activeModId)
     : null;
 
   useEffect(() => {

applications/browser-extension/src/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames.ts

@@ -31,7 +31,7 @@ import { useDispatch, useSelector } from "react-redux";
 import {
   selectActiveModComponentFormState,
   selectActiveModComponentId,
-  selectCurrentModId,
+  selectActiveModId,
   selectEditorUpdateKey,
   selectGetModDraftStateForModId,
 } from "@/pageEditor/store/editor/editorSelectors";
@@ -113,7 +113,7 @@ function updateDraftModInstance() {
     const state = getState();
 
     const activeModComponentId = selectActiveModComponentId(state);
-    const modId = selectCurrentModId(state);
+    const modId = selectActiveModId(state);
 
     if (!modId) {
       // Skip if the modId has somehow become null before the microtask for this async method got scheduled
@@ -175,7 +175,7 @@ function updateDraftModInstance() {
  */
 function useRegisterDraftModInstanceOnAllFrames(): void {
   const dispatch = useDispatch<AppDispatch>();
-  const modId = useSelector(selectCurrentModId);
+  const modId = useSelector(selectActiveModId);
   const editorUpdateKey = useSelector(selectEditorUpdateKey);
 
   assertNotNullish(modId, "modId is required");

applications/browser-extension/src/pageEditor/modListingPanel/ModListItem.tsx

@@ -29,7 +29,7 @@ import {
 import { useDispatch, useSelector } from "react-redux";
 import cx from "classnames";
 import {
-  selectActiveModComponentFormState,
+  selectActiveModComponentId,
   selectActiveModId,
   selectDirtyMetadataForModId,
   selectExpandedModId,
@@ -47,24 +47,19 @@ const ModListItem: React.FC<
   const dispatch = useDispatch();
   const activeModId = useSelector(selectActiveModId);
   const expandedModId = useSelector(selectExpandedModId);
-  const activeModComponentFormState = useSelector(
-    selectActiveModComponentFormState,
-  );
+  const activeModComponentId = useSelector(selectActiveModComponentId);
 
   const { id: modId, name: savedName, version: activatedVersion } = modMetadata;
 
-  const isActive = activeModId === modId;
+  const isModActive = activeModId === modId;
+  const isModComponentActive = activeModComponentId != null;
   const isExpanded = expandedModId === modId;
 
   // TODO: Fix this so it pulls from registry, after registry single-item-api-fetch is implemented
   //        (See: https://github.com/pixiebrix/pixiebrix-extension/issues/7184)
   const { data: modDefinition } = useGetModDefinitionQuery({ modId });
   const latestModVersion = modDefinition?.metadata?.version;
 
-  // Set the alternate background if a mod component in this mod is active
-  const hasModBackground =
-    activeModComponentFormState?.modMetadata.id === modId;
-
   const dirtyName = useSelector(selectDirtyMetadataForModId(modId))?.name;
   const name = dirtyName ?? savedName ?? "Loading...";
 
@@ -81,17 +76,16 @@ const ModListItem: React.FC<
         eventKey={modId}
         as={ListGroup.Item}
         className={cx(styles.root, "list-group-item-action", {
-          [styles.modBackground ?? ""]: hasModBackground,
+          // Set the alternate background if a mod component in this mod is active
+          [styles.modBackground ?? ""]: isModActive && isModComponentActive,
         })}
         tabIndex={0} // Avoid using `button` because this item includes more buttons #2343
-        active={isActive}
-        key={`mod-${modId}`}
+        active={isModActive && !isModComponentActive}
+        key={modId}
         onClick={() => {
           dispatch(actions.setActiveModId(modId));
           // Collapse if the user clicks the mod item when it's already active/selected in the listing pane
-          dispatch(
-            actions.setExpandedModId(isExpanded && isActive ? null : modId),
-          );
+          dispatch(actions.setExpandedModId(isExpanded ? null : modId));
         }}
       >
         <span className={styles.icon}>

applications/browser-extension/src/pageEditor/modListingPanel/ModSidebarListItems.tsx

@@ -77,25 +77,33 @@ const ModSidebarListItems: React.FunctionComponent = () => {
     ],
   );
 
-  const listItems = filteredSidebarItems.map((sidebarItem) => {
-    const { modMetadata, modComponents } = sidebarItem;
+  const listItems = useMemo(
+    () =>
+      filteredSidebarItems.map((sidebarItem) => {
+        const { modMetadata, modComponents } = sidebarItem;
 
-    return (
-      <ModListItem key={modMetadata.id} modMetadata={modMetadata}>
-        {modComponents.map((modComponentSidebarItem) => (
-          <ModComponentListItem
-            key={getDraftModComponentId(modComponentSidebarItem)}
-            modComponentSidebarItem={modComponentSidebarItem}
-            availableActivatedModComponentIds={
-              availableActivatedModComponentIds
-            }
-            availableDraftModComponentIds={availableDraftModComponentIds}
-            isNested
-          />
-        ))}
-      </ModListItem>
-    );
-  });
+        return (
+          <ModListItem key={modMetadata.id} modMetadata={modMetadata}>
+            {modComponents.map((modComponentSidebarItem) => (
+              <ModComponentListItem
+                key={getDraftModComponentId(modComponentSidebarItem)}
+                modComponentSidebarItem={modComponentSidebarItem}
+                availableActivatedModComponentIds={
+                  availableActivatedModComponentIds
+                }
+                availableDraftModComponentIds={availableDraftModComponentIds}
+                isNested
+              />
+            ))}
+          </ModListItem>
+        );
+      }),
+    [
+      availableActivatedModComponentIds,
+      availableDraftModComponentIds,
+      filteredSidebarItems,
+    ],
+  );
 
   return (
     <>

applications/browser-extension/src/pageEditor/modListingPanel/modals/CreateModModal.tsx

@@ -27,7 +27,7 @@ import { useDispatch, useSelector } from "react-redux";
 import {
   getModalDataSelector,
   selectActiveModComponentFormState,
-  selectCurrentModId,
+  selectActiveModId,
   selectEditorModalVisibilities,
   selectModMetadataMap,
 } from "@/pageEditor/store/editor/editorSelectors";
@@ -138,8 +138,8 @@ const CreateModModalBody: React.FC<{ onHide: () => void }> = ({ onHide }) => {
   const dispatch = useDispatch();
   const isMounted = useIsMounted();
 
-  const currentModId = useSelector(selectCurrentModId);
-  assertNotNullish(currentModId, "Expected mod or mod component to be active");
+  const activeModId = useSelector(selectActiveModId);
+  assertNotNullish(activeModId, "Expected mod or mod component to be active");
 
   const activeModComponentFormState = useSelector(
     selectActiveModComponentFormState,
@@ -156,11 +156,11 @@ const CreateModModalBody: React.FC<{ onHide: () => void }> = ({ onHide }) => {
   const { createModFromComponent } = useCreateModFromModComponent();
 
   const { data: modDefinition, isFetching: isModDefinitionFetching } =
-    useOptionalModDefinition(currentModId);
+    useOptionalModDefinition(activeModId);
 
   const formSchema = useFormSchema();
 
-  const initialFormState = useInitialFormState(currentModId);
+  const initialFormState = useInitialFormState(activeModId);
 
   const onSubmit: OnSubmit<ModMetadataFormState> = async (values, helpers) => {
     if (isModDefinitionFetching) {
@@ -185,10 +185,10 @@ const CreateModModalBody: React.FC<{ onHide: () => void }> = ({ onHide }) => {
       ) {
         // Handle "Save As" case where the mod is unsaved or the user no longer has access to the mod definition
         assertNotNullish(
-          currentModId,
+          activeModId,
           "Expected mod to be selected in the editor",
         );
-        await createModFromUnsavedMod(currentModId, values);
+        await createModFromUnsavedMod(activeModId, values);
       } else {
         await createModFromMod(modDefinition, values, modalData);
       }

applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveAsNewModModal.tsx

@@ -20,7 +20,7 @@ import { useDispatch, useSelector } from "react-redux";
 import { actions } from "@/pageEditor/store/editor/editorSlice";
 import { Button, Modal } from "react-bootstrap";
 import {
-  selectCurrentModId,
+  selectActiveModId,
   selectEditorModalVisibilities,
 } from "@/pageEditor/store/editor/editorSelectors";
 import { useOptionalModDefinition } from "@/modDefinitions/modDefinitionHooks";
@@ -32,7 +32,7 @@ const SaveAsNewModModal: React.FC = () => {
     selectEditorModalVisibilities,
   );
 
-  const modId = useSelector(selectCurrentModId);
+  const modId = useSelector(selectActiveModId);
   const { data: mod, isFetching } = useOptionalModDefinition(modId);
   const modName = mod?.metadata?.name ?? "this mod";
 

applications/browser-extension/src/pageEditor/store/editor/editorInvariantMiddleware.ts

@@ -18,15 +18,15 @@
 import type { AnyAction, Dispatch, Middleware } from "@reduxjs/toolkit";
 import { type EditorRootState } from "@/pageEditor/store/editor/pageEditorTypes";
 import {
-  selectActiveModComponentId,
+  selectActiveModComponentFormState,
   selectActiveModId,
   selectAllDeletedModComponentIds,
-  selectCurrentModId,
   selectExpandedModId,
   selectModComponentFormStates,
 } from "@/pageEditor/store/editor/editorSelectors";
 import type { EmptyObject } from "type-fest";
 import { uniqBy } from "lodash";
+import { isInternalRegistryId } from "@/utils/registryUtils";
 
 class InvariantViolationError extends Error {
   override name = "InvariantViolationError";
@@ -48,17 +48,28 @@ class InvariantViolationError extends Error {
 // XXX: in production, should we be attempting to auto-fix these invariants?
 export function assertEditorInvariants(state: EditorRootState): void {
   // Assert that a mod and mod component item cannot be selected at the same time
-  if (selectActiveModId(state) && selectActiveModComponentId(state)) {
+  const activeModId = selectActiveModId(state);
+  const activeModComponent = selectActiveModComponentFormState(state);
+
+  if (
+    activeModId &&
+    activeModComponent &&
+    activeModId !== activeModComponent?.modMetadata.id &&
+    // When saving, the activeModId and activeModComponent.modMetadata.id aren't updated at the same time.
+    !isInternalRegistryId(activeModId)
+  ) {
+    // Should we dispatch(actions.setActiveModComponentId(null))
+    // Would need to change the behavior of the action to handle null
     throw new InvariantViolationError(
-      "activeModId and activeModComponentId are both set",
+      "activeModComponent is not a part of the activeMod",
     );
   }
 
   // Assert that the expanded mod must correspond to the selected mod or mod component
   const expandedModId = selectExpandedModId(state);
-  if (expandedModId && selectCurrentModId(state) !== expandedModId) {
+  if (expandedModId && activeModId !== expandedModId) {
     throw new InvariantViolationError(
-      "expandedModId does not match active mod/mod component",
+      "expandedModId does not match active mod",
     );
   }
 

applications/browser-extension/src/pageEditor/store/editor/editorSelectors/editorDataPanelSelectors.ts

@@ -21,7 +21,7 @@ import type { DataPanelTabKey } from "@/pageEditor/tabs/editTab/dataPanel/dataPa
 import { createSelector } from "@reduxjs/toolkit";
 import type { DataPanelTabUIState } from "@/pageEditor/store/editor/uiStateTypes";
 import { selectActiveBrickConfigurationUIState } from "@/pageEditor/store/editor/editorSelectors/editorPipelineSelectors";
-import { selectCurrentModId } from "@/pageEditor/store/editor/editorSelectors/editorModSelectors";
+import { selectActiveModId } from "@/pageEditor/store/editor/editorSelectors/editorNavigationSelectors";
 
 export const selectIsDataPanelExpanded = ({ editor }: EditorRootState) =>
   editor.isDataPanelExpanded;
@@ -55,9 +55,9 @@ export function selectNodeDataPanelTabState(
  */
 export const selectCurrentFindInModQuery = createSelector(
   ({ editor }: EditorRootState) => editor.findInModQueryByModId,
-  selectCurrentModId,
+  selectActiveModId,
   (findInModQueryByModId, modId) => {
-    assertNotNullish(modId, "Expected currentModId");
+    assertNotNullish(modId, "Expected activeModId");
     // eslint-disable-next-line security/detect-object-injection -- registry id
     return findInModQueryByModId[modId] ?? { query: "" };
   },

applications/browser-extension/src/pageEditor/store/editor/editorSelectors/editorModSelectors.ts

@@ -23,13 +23,11 @@ import { selectModInstances } from "@/store/modComponents/modInstanceSelectors";
 import mapModDefinitionToModMetadata from "@/modDefinitions/util/mapModDefinitionToModMetadata";
 import { normalizeModOptionsDefinition } from "@/utils/modUtils";
 import {
-  selectActiveModComponentFormState,
   selectGetModComponentFormStateByModComponentId,
   selectIsModComponentDirtyById,
   selectModComponentFormStates,
   selectNotDeletedActivatedModComponents,
 } from "@/pageEditor/store/editor/editorSelectors/editorModComponentSelectors";
-import { selectActiveModId } from "@/pageEditor/store/editor/editorSelectors/editorNavigationSelectors";
 import type { ModMetadata } from "@/types/modComponentTypes";
 import type { UUID } from "@/types/stringTypes";
 import { assertNotNullish } from "@/utils/nullishUtils";
@@ -39,19 +37,6 @@ import {
   collectModVariablesDefinition,
 } from "@/store/modComponents/modComponentUtils";
 
-/**
- * Select the mod id associated with the selected mod package or mod component. Should be used if the caller doesn't
- * need to know if the mod item or one of its components is selected.
- * @see selectActiveModId
- * @see selectExpandedModId
- */
-export const selectCurrentModId = createSelector(
-  selectActiveModId,
-  selectActiveModComponentFormState,
-  (activeModId, activeModComponentFormState) =>
-    activeModId ?? activeModComponentFormState?.modMetadata.id,
-);
-
 ///
 /// MOD METADATA
 ///

applications/browser-extension/src/pageEditor/store/editor/editorSelectors/editorNavigationSelectors.ts

@@ -29,9 +29,8 @@ export const selectActiveModComponentId = ({ editor }: EditorRootState) => {
 };
 
 /**
- * Select the id of the mod being edited. NOTE: is null when editing a mod component within the mod.
+ * Select the id of the mod being edited.
  * @see selectModId
- * @see selectCurrentModId
  */
 export const selectActiveModId = ({ editor }: EditorRootState) =>
   editor.activeModId;

applications/browser-extension/src/pageEditor/store/editor/editorSelectors/editorPipelineSelectors.ts

@@ -73,9 +73,9 @@ export const selectActiveNodeInfo = createSelector(
 const activeModComponentNodeInfoSelector = createSelector(
   selectActiveBrickPipelineUIState,
   (_state: EditorRootState, instanceId: UUID) => instanceId,
-  (uiState: BrickPipelineUIState, instanceId: UUID) =>
+  (uiState, instanceId) =>
     // eslint-disable-next-line security/detect-object-injection -- using a node uuid
-    uiState.pipelineMap[instanceId],
+    uiState?.pipelineMap[instanceId],
 );
 
 export const selectActiveModComponentNodeInfo =
@@ -103,21 +103,21 @@ export const selectActiveNodeEventData = createSelector(
 
 export const selectPipelineMap = createSelector(
   selectActiveBrickPipelineUIState,
-  (uiState: BrickPipelineUIState) => uiState?.pipelineMap,
+  (uiState) => uiState?.pipelineMap,
 );
 
 export const selectCollapsedNodes = createSelector(
   selectActiveBrickPipelineUIState,
-  (brickPipelineUIState: BrickPipelineUIState) =>
-    Object.entries(brickPipelineUIState.nodeUIStates)
+  (brickPipelineUIState) =>
+    Object.entries(brickPipelineUIState?.nodeUIStates ?? {})
       .map(([nodeId, { collapsed }]) => (collapsed ? nodeId : null))
       .filter((nodeId) => nodeId != null),
 );
 
 const parentNodeInfoSelector = createSelector(
   selectActiveBrickPipelineUIState,
   (_state: EditorRootState, nodeId: UUID) => nodeId,
-  (brickPipelineUIState: BrickPipelineUIState, nodeId: UUID) => {
+  (brickPipelineUIState, nodeId) => {
     if (brickPipelineUIState == null) {
       return null;
     }