Require version on ModDefinition.metadata (#9264)

src/extensionConsole/pages/mods/GetStartedView.stories.tsx

@@ -16,7 +16,7 @@
  */
 
 import React from "react";
-import { type ComponentStory, type ComponentMeta } from "@storybook/react";
+import { type ComponentMeta, type ComponentStory } from "@storybook/react";
 import { configureStore } from "@reduxjs/toolkit";
 import { persistReducer } from "redux-persist";
 import modsPageSlice, {
@@ -28,13 +28,11 @@ import { Provider } from "react-redux";
 import { authSlice, persistAuthConfig } from "@/auth/authSlice";
 import { rest } from "msw";
 import { modDefinitionsSlice } from "@/modDefinitions/modDefinitionsSlice";
-import { type ModDefinition } from "@/types/modDefinitionTypes";
 import { valueToAsyncCacheState } from "@/utils/asyncStateUtils";
-import { type ApiVersion } from "@/types/runtimeTypes";
-import { type Timestamp } from "@/types/stringTypes";
-import { validateRegistryId } from "@/types/helpers";
-import { DefinitionKinds } from "@/types/registryTypes";
 import { API_PATHS } from "@/data/service/urlPaths";
+import { publicSharingDefinitionFactory } from "@/testUtils/factories/registryFactories";
+import { modDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories";
+import { Milestones } from "@/data/model/UserMilestone";
 
 export default {
   title: "ModsPage/GetStartedView",
@@ -56,33 +54,22 @@ function optionsStore(initialState?: UnknownObject) {
   });
 }
 
-const testRecipe = {
-  metadata: {
-    id: validateRegistryId("@pixiebrix/test-blueprint"),
-    name: "Test Blueprint",
-  },
-  extensionPoints: [],
-  sharing: {
-    public: true,
-    organizations: [],
-  },
-  updated_at: "2022-01-01T00:00:00Z" as Timestamp,
-  kind: DefinitionKinds.MOD,
-  apiVersion: "v3" as ApiVersion,
-} as ModDefinition;
+const testMod = modDefinitionFactory({
+  sharing: publicSharingDefinitionFactory(),
+});
 
 const Template: ComponentStory<typeof GetStartedView> = (args) => (
   <Provider
     store={optionsStore({
       auth: {
         milestones: [
           {
-            key: "first_time_public_blueprint_install",
-            metadata: { blueprintId: testRecipe.metadata.id },
+            key: Milestones.FIRST_TIME_PUBLIC_MOD_ACTIVATION,
+            metadata: { blueprintId: testMod.metadata.id },
           },
         ],
       },
-      recipes: valueToAsyncCacheState([testRecipe]),
+      recipes: valueToAsyncCacheState([testMod]),
     })}
   >
     <GetStartedView {...args} />
@@ -95,13 +82,13 @@ Default.args = {
   height: 500,
 };
 
-export const ActivateBlueprint = Template.bind({});
-ActivateBlueprint.args = {
+export const ActivateMod = Template.bind({});
+ActivateMod.args = {
   width: 800,
   height: 500,
 };
 
-ActivateBlueprint.parameters = {
+ActivateMod.parameters = {
   msw: {
     handlers: [
       rest.get(
@@ -111,8 +98,8 @@ ActivateBlueprint.parameters = {
             context.json([
               {
                 package: {
-                  name: testRecipe.metadata.id,
-                  verbose_name: testRecipe.metadata.name,
+                  name: testMod.metadata.id,
+                  verbose_name: testMod.metadata.name,
                 },
               },
             ]),

src/pageEditor/panes/save/saveHelpers.test.ts

@@ -48,7 +48,10 @@ import {
   type UnsavedModDefinition,
 } from "@/types/modDefinitionTypes";
 import { type SerializedModComponent } from "@/types/modComponentTypes";
-import { modComponentFactory } from "@/testUtils/factories/modComponentFactories";
+import {
+  modComponentFactory,
+  modMetadataFactory,
+} from "@/testUtils/factories/modComponentFactories";
 import {
   defaultModDefinitionFactory,
   innerStarterBrickModDefinitionFactory,
@@ -356,13 +359,12 @@ describe("replaceModComponent round trip", () => {
 
     jest.mocked(lookupStarterBrick).mockResolvedValue({
       ...modDefinition.definitions!.extensionPoint!,
-      metadata: {
+      metadata: modMetadataFactory({
         id: calculateInnerRegistryId(
           modDefinition.definitions!.extensionPoint!,
         ),
         name: "Internal Starter Brick",
-        version: normalizeSemVerString("1.0.0"),
-      },
+      }),
     } as any);
 
     const modComponentFormState =

src/pageEditor/panes/save/saveHelpers.ts

@@ -19,11 +19,12 @@ import {
   DefinitionKinds,
   type InnerDefinitionRef,
   type InnerDefinitions,
-  type Metadata,
   type RegistryId,
+  type VersionedMetadata,
 } from "@/types/registryTypes";
 import {
   isInnerDefinitionRegistryId,
+  normalizeSemVerString,
   PACKAGE_REGEX,
   validateRegistryId,
 } from "@/types/helpers";
@@ -160,7 +161,7 @@ function deleteUnusedStarterBrickDefinitions(
  */
 export function replaceModComponent(
   sourceMod: ModDefinition,
-  modMetadata: Metadata,
+  modMetadata: VersionedMetadata,
   activatedModComponents: ModComponentBase[],
   newModComponent: ModComponentFormState,
 ): UnsavedModDefinition {
@@ -313,6 +314,7 @@ const emptyModDefinition: UnsavedModDefinition = {
   metadata: {
     id: "" as RegistryId,
     name: "",
+    version: normalizeSemVerString("1.0.0"),
   },
   extensionPoints: [],
   definitions: {},
@@ -323,7 +325,7 @@ const emptyModDefinition: UnsavedModDefinition = {
 /**
  * Create a copy of `sourceMod` (if provided) with given mod metadata, mod options, and mod components.
  *
- * NOTE: the caller is responsible for updating an starter brick package (i.e., that has its own version). This method
+ * NOTE: the caller is responsible for updating a starter brick package (i.e., that has its own version). This method
  * only handles the starter brick if it's an inner definition
  *
  * @param sourceMod the original mod definition, or undefined for new mods

src/pageEditor/starterBricks/base.ts

@@ -347,6 +347,7 @@ export function baseSelectStarterBrick(
       id: metadata.id,
       // The server requires the version to save the brick, even though it's not marked as required
       // in the front-end schemas
+      // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/9265 -- mark version as required
       version: metadata.version ?? normalizeSemVerString("1.0.0"),
       name: metadata.name,
       // The server requires the description to save the brick, even though it's not marked as required

src/pageEditor/store/editor/pageEditorTypes.ts

@@ -22,7 +22,7 @@ import { type SettingsRootState } from "@/store/settings/settingsTypes";
 import { type RuntimeRootState } from "@/pageEditor/store/runtime/runtimeSliceTypes";
 import { type StarterBrickType } from "@/types/starterBrickTypes";
 import { type UUID } from "@/types/stringTypes";
-import { type Metadata, type RegistryId } from "@/types/registryTypes";
+import { type RegistryId, type VersionedMetadata } from "@/types/registryTypes";
 import { type BrickConfig, type PipelineFlavor } from "@/bricks/types";
 import { type BrickPipelineUIState } from "@/pageEditor/store/editor/uiStateTypes";
 import { type AnalysisRootState } from "@/analysis/analysisTypes";
@@ -70,7 +70,7 @@ export enum ModalKey {
 }
 
 export type ModMetadataFormState = Pick<
-  Metadata,
+  VersionedMetadata,
   "id" | "name" | "version" | "description"
 >;
 

src/sidebar/activateMod/ActivateModPanel.test.tsx

@@ -54,6 +54,7 @@ import { registryIdFactory } from "@/testUtils/factories/stringFactories";
 import { propertiesToSchema } from "@/utils/schemaUtils";
 import { INTEGRATIONS_BASE_SCHEMA_URL } from "@/integrations/constants";
 import { API_PATHS } from "@/data/service/urlPaths";
+import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories";
 
 jest.mock("@/modDefinitions/modDefinitionHooks");
 jest.mock("@/sidebar/sidebarSelectors");
@@ -147,10 +148,10 @@ function setupMocksAndRender(
 ) {
   modDefinition = defaultModDefinitionFactory({
     ...modDefinitionOverride,
-    metadata: {
+    metadata: modMetadataFactory({
       id: modRegistryId,
       name: "Test Mod",
-    },
+    }),
   });
   useRequiredModDefinitionsMock.mockReturnValue(
     valueToAsyncCacheState([modDefinition]),

src/testUtils/factories/metadataFactory.ts

@@ -16,10 +16,10 @@
  */
 
 import { define } from "cooky-cutter";
-import { type Metadata } from "@/types/registryTypes";
+import type { VersionedMetadata } from "@/types/registryTypes";
 import { validateRegistryId, normalizeSemVerString } from "@/types/helpers";
 
-export const metadataFactory = define<Metadata>({
+export const metadataFactory = define<VersionedMetadata>({
   id: (n: number) => validateRegistryId(`test/mod-${n}`),
   name: (n: number) => `Mod ${n}`,
   description: "Mod generated from factory",

src/types/modComponentTypes.ts

@@ -20,9 +20,9 @@ import { type Except } from "type-fest";
 import {
   type InnerDefinitionRef,
   type InnerDefinitions,
-  type Metadata,
   type RegistryId,
   type Sharing,
+  type VersionedMetadata,
 } from "@/types/registryTypes";
 import { type Timestamp, type UUID } from "@/types/stringTypes";
 import {
@@ -50,7 +50,7 @@ import { type ModVariablesDefinition } from "@/types/modDefinitionTypes";
  */
 // XXX: previously we didn't export because the usage was clearer as ModComponentBase[_recipe]. However, the ergonomics
 // of (ModMetadata | undefined) were bad to handle with strict null checks
-export type ModMetadata = Metadata & {
+export type ModMetadata = VersionedMetadata & {
   /**
    * `undefined` for mods that were activated prior to the field being added
    */

src/types/modDefinitionTypes.ts

@@ -22,6 +22,7 @@ import {
   type InnerDefinitions,
   type DefinitionKinds,
   type RegistryId,
+  type VersionedMetadata,
 } from "@/types/registryTypes";
 import { type Schema } from "@/types/schemaTypes";
 import { type Timestamp, type UUID } from "@/types/stringTypes";
@@ -120,6 +121,8 @@ export type HydratedModComponentDefinition = ModComponentDefinition & {
  */
 export interface UnsavedModDefinition extends Definition {
   kind: typeof DefinitionKinds.MOD;
+  // Strengthen Metadata.version to be required
+  metadata: VersionedMetadata;
   extensionPoints: ModComponentDefinition[];
   definitions?: InnerDefinitions;
   options?: ModOptionsDefinition;

src/types/registryTypes.ts

@@ -17,7 +17,7 @@
 
 import { type UUID } from "@/types/stringTypes";
 import { type ApiVersion } from "@/types/runtimeTypes";
-import { type Tagged, type ValueOf } from "type-fest";
+import { type SetRequired, type Tagged, type ValueOf } from "type-fest";
 import { type Schema } from "@/types/schemaTypes";
 import { type FeatureFlag } from "@/auth/featureFlags";
 
@@ -84,6 +84,7 @@ export type Metadata = {
    *
    * Currently optional because it defaults to the browser extension version for bricks defined in JS.
    */
+  // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/9265 -- require version for all metadata
   readonly version?: SemVerString;
 
   /**
@@ -94,6 +95,21 @@ export type Metadata = {
   readonly extensionVersion?: SemVerString;
 };
 
+/**
+ * Registry item metadata definition shape with required version.
+ *
+ * Introduced in 2.1.5 to strengthen the type of `Metadata.version` for ModDefinitions.
+ *
+ * Use PackageInstance instead if expecting a package instance, e.g., `Brick`, `StarterBrick`, `Integration`.
+ *
+ * @see ModDefinition.metadata
+ * @see Metadata
+ * @see PackageInstance
+ * @since 2.1.5
+ */
+// TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/9265 -- require version for all metadata
+export type VersionedMetadata = SetRequired<Metadata, "version">;
+
 /**
  * Interface for registry package instances, i.e., `Brick`, `StarterBrick`, and `Integration`.
  *

src/utils/deploymentUtils.ts

@@ -93,8 +93,7 @@ export const makeUpdatedFilter =
     if (
       packageMatch &&
       gte(
-        // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- version is present for persisted mods
-        packageMatch.definition.metadata.version!,
+        packageMatch.definition.metadata.version,
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- deployment package is checked above
         deployment.package.version!,
       )