Add struct utils for validating JSON objects with optional values #136

Mrtenz · 2023-08-31T09:34:11Z

This adds two new structs:

object: Similar to the object struct from Superstruct, but infers optional values as key?: value instead of key?: value | undefined.
exactOptional: Similar to the optional struct from Superstruct, but does not allow undefined values.

This lets us use proper runtime validation for these objects, and also type these objects in a way that they don't allow undefined values, without the need to modify the inferred type.

I've updated all the existing structs, with the exception of PendingJsonRpcResponseStruct, since it seems like undefined is valid for this type, at least according to the tests.

legobeat · 2023-08-31T11:01:30Z

typings diff vs current main:

diff -x '*.map' -wr -U1 dist-a00dfc9d/types/json.d.ts dist-4bc04f/types/json.d.ts
--- dist-a00dfc9d/types/json.d.ts
+++ dist-4bc04f/types/json.d.ts
@@ -1,2 +1,3 @@
 import type { Infer, Struct } from 'superstruct';
+import type { ObjectSchema, OmitBy, PickBy, Simplify } from 'superstruct/dist/utils';
 import type { AssertionErrorConstructor } from './assert';
@@ -9,2 +10,43 @@
 /**
+ * A helper type to make properties with `undefined` in their type optional, but
+ * not `undefined` itself.
+ *
+ * @example
+ * ```ts
+ * type Foo = JsonOptional<{ foo: string | undefined }>;
+ * // Foo is equivalent to { foo?: string }
+ * ```
+ */
+export declare type JsonOptional<Schema extends Record<string, unknown>> = {
+    [Key in keyof PickBy<Schema, undefined>]?: Exclude<Schema[Key], undefined>;
+} & {
+    [Key in keyof OmitBy<Schema, undefined>]: Schema[Key];
+};
+/**
+ * A JSON object type. This is used by the `json` struct. This uses the
+ * {@link JsonOptional} helper to make properties with `undefined` in their type
+ * optional, but not `undefined` itself.
+ */
+export declare type JsonObjectType<Schema extends ObjectSchema> = Simplify<JsonOptional<{
+    [Key in keyof Schema]: Infer<Schema[Key]>;
+}>>;
+/**
+ * A struct to check if the given value is a valid JSON object.
+ *
+ * @param schema - The schema of the JSON object.
+ * @returns A struct to check if the given value is a valid JSON object.
+ */
+export declare const jsonObject: <Schema extends ObjectSchema>(schema: Schema) => Struct<Simplify<JsonOptional<{ [Key in keyof Schema]: Infer<Schema[Key]>; }>>, unknown>;
+/**
+ * A struct to check if the given value is a valid JSON object, or not present.
+ * This means that it allows an object which does not have the property, or an
+ * object which has the property and is valid, but not an object which has the
+ * property set to `undefined`.
+ *
+ * @param struct - The struct to check the value against, if present.
+ * @returns A struct to check if the given value is a valid JSON object, or not
+ * present.
+ */
+export declare const jsonOptional: <Type, Schema>(struct: Struct<Type, Schema>) => Struct<Type | undefined, null>;
+/**
  * A struct to check if the given value is a valid JSON-serializable value.
@@ -68,12 +110,7 @@
 export declare const JsonRpcErrorStruct: Struct<{
+    data?: Json;
+    stack?: string;
     code: number;
     message: string;
-    data?: Json | undefined;
-    stack?: string | undefined;
-}, {
-    code: Struct<number, null>;
-    message: Struct<string, null>;
-    data: Struct<Json | undefined, unknown>;
-    stack: Struct<string | undefined, null>;
-}>;
+}, unknown>;
 /**
@@ -93,2 +130,3 @@
 export declare const JsonRpcRequestStruct: Struct<{
+    params?: Json[] | Record<string, Json>;
     id: string | number | null;
@@ -96,11 +134,5 @@
     jsonrpc: "2.0";
-    params?: Json[] | Record<string, Json> | undefined;
-}, {
-    id: Struct<string | number | null, null>;
-    jsonrpc: Struct<"2.0", "2.0">;
-    method: Struct<string, null>;
-    params: Struct<Json[] | Record<string, Json> | undefined, null>;
-}>;
-export declare type InferWithParams<Type extends Struct<any>, Params extends JsonRpcParams> = Omit<Infer<Type>, 'params'> & {
-    params?: Exclude<Params, undefined>;
+}, unknown>;
+export declare type InferWithParams<Type extends Struct<any>, Params extends JsonRpcParams> = Infer<Type> & {
+    params?: Params;
 };
@@ -111,10 +143,6 @@
 export declare const JsonRpcNotificationStruct: Struct<{
+    params?: Json[] | Record<string, Json>;
     method: string;
     jsonrpc: "2.0";
-    params?: Json[] | Record<string, Json> | undefined;
-}, {
-    jsonrpc: Struct<"2.0", "2.0">;
-    method: Struct<string, null>;
-    params: Struct<Json[] | Record<string, Json> | undefined, null>;
-}>;
+}, unknown>;
 /**
@@ -161,6 +189,6 @@
     error?: {
+        data?: Json;
+        stack?: string;
         code: number;
         message: string;
-        data?: Json | undefined;
-        stack?: string | undefined;
     } | undefined;
@@ -171,12 +199,7 @@
     error: Struct<{
+        data?: Json;
+        stack?: string;
         code: number;
         message: string;
-        data?: Json | undefined;
-        stack?: string | undefined;
-    } | undefined, {
-        code: Struct<number, null>;
-        message: Struct<string, null>;
-        data: Struct<Json | undefined, unknown>;
-        stack: Struct<string | undefined, null>;
-    }>;
+    } | undefined, unknown>;
 }>;
@@ -192,7 +215,3 @@
     result: Json;
-}, {
-    id: Struct<string | number | null, null>;
-    jsonrpc: Struct<"2.0", "2.0">;
-    result: Struct<Json, unknown>;
-}>;
+}, unknown>;
 /**
@@ -207,7 +226,3 @@
     jsonrpc: "2.0";
-}, {
-    id: Struct<string | number | null, null>;
-    jsonrpc: Struct<"2.0", "2.0">;
-    error: Struct<JsonRpcError, unknown>;
-}>;
+}, unknown>;
 /**

src/json.ts

danroc · 2023-09-01T20:56:49Z

src/json.ts

+export type ObjectOptional<Schema extends Record<string, unknown>> = {
+  [Key in keyof Schema as Schema[Key] extends ExactOptionalGuard
+    ? Key
+    : never]?: Schema[Key] extends ExactOptionalGuard & infer Original


IMO, if exactOptionalPropertyTypes is disabled, the type should be left as

{ foo: string }

instead of

{ foo?: string | undefined }

I'm not 100% sure about it. It would make it more difficult to work with if exactOptionalPropertyTypes is disabled (which it is currently on the Snaps repo).

legobeat · 2023-09-06T00:05:02Z

Resolves #139

mcmire

I believe I understand the general gist of this PR but I did have some questions.

As a side note I'm starting to think that the struct utils should be split off at some point, or at least put under some kind of namespace. The json file has grown pretty large, and object is a pretty generic function name that doesn't really indicate how it ought to be used. Perhaps once we get utils into the core monorepo we can start to think about how best to accomplish that.

src/json.test.ts

mcmire · 2023-10-19T18:12:36Z

src/json.ts

+ * // Foo is equivalent to { foo?: string }
+ * ```
+ */
+export type ObjectOptional<Schema extends Record<string, unknown>> = {


It seems like this type essentially converts properties that should be optional (based on the fact that they can be undefined) so that they actually are. I don't know, however, that I would be able to guess that from this name. I see that Superstruct already has a type called Optionalize. What does that type do that this doesn't do? What do you think about a name like NormalizeOptionals? Then again, if this name is patterned after something else I'm not aware of, let me know.

Superstruct's Optionalize doesn't work with exact optionals. This is basically a copy of that, but also checks if the property has a ExactOptionalGuard, in which case the type will be inferred as key?: Type, instead of key?: Type | undefined. It's a bit hard to read 😅 I don't expect consumers of utils to use this type directly, so I don't think the name really matters?

Gotcha. Do you think it's something we want to export in that case then? If you plan on using it in Snaps, that's fine, just curious.

Er, I guess that's a silly question. I can see how they would be useful, so never mind!

I think it's good practice to export any "helper" types used. There are rare cases (usually in tests) where you may need to reproduce a certain type, and it helps if those are exported.

src/json.ts

mcmire · 2023-10-19T18:49:47Z

src/json.ts

+
+declare const exactOptionalSymbol: unique symbol;
+type ExactOptionalGuard = {
+  _exactOptionalGuard?: typeof exactOptionalSymbol;


Can you explain how this type is used? I see that it's referred to above, but where does this property get set?

This property is never set: It's only used for inferring types. It's added to the type of the exact optional struct, and used by ObjectOptional to infer the proper type.

Okay. This is advanced! I can see how that works though.

Yeah it's definitely advanced. We use a unique symbol type to ensure that the type cannot be replicated in any way. unique symbols are guaranteed to be unique:

declare const a: unique symbol; declare const b: unique symbol; // TS2367: This condition will always return false since the types typeof a and typeof b have // no overlap. a === b;

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

mcmire

I'm good with these changes — they seem useful! The leftover comments I have are nitpicks.

mcmire

Looks good!

Mrtenz marked this pull request as ready for review August 31, 2023 09:51

Mrtenz requested a review from a team as a code owner August 31, 2023 09:51

This comment was marked as resolved.

Sign in to view

legobeat mentioned this pull request Aug 31, 2023

TEST: patch @metamask/utils 4bc04f1e61d2589e139fc2dd1b5dfba14dc1324b MetaMask/core#1646

Closed

legobeat previously approved these changes Aug 31, 2023

View reviewed changes

Mrtenz dismissed legobeat’s stale review via a0d5748 August 31, 2023 17:20

danroc mentioned this pull request Sep 1, 2023

Add support to exactOptional properties #138

Closed

danroc reviewed Sep 1, 2023

View reviewed changes

src/json.ts Outdated Show resolved Hide resolved

danroc reviewed Sep 1, 2023

View reviewed changes

legobeat mentioned this pull request Sep 6, 2023

Forbid undefined values on request and response objects #139

Open

Mrtenz added 10 commits October 19, 2023 19:50

Add struct utils for validating JSON objects with optional values

66bd785

Add type tests

beb5922

Update existing structs to use jsonObject struct

347543d

Add test for coverage

4a26919

Fix type of InferWithParams

506bb72

Rename to object and exactOptional

678c603

Fix type test

b5ecb4b

Remove unused imports

12ad29c

Rename test names

c1663d4

Address review comments

0c35eeb

Mrtenz force-pushed the mrtenz/json-object-validation branch from 035998d to 0c35eeb Compare October 19, 2023 17:50

mcmire reviewed Oct 19, 2023

View reviewed changes

Mrtenz and others added 3 commits October 19, 2023 21:45

Update src/json.test.ts

90a8a51

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

Update src/json.ts

85f24ea

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

Update snapshots

44cf78f

mcmire previously approved these changes Oct 19, 2023

View reviewed changes

Move some tests

c15fea9

Mrtenz dismissed mcmire’s stale review via c15fea9 October 19, 2023 20:16

mcmire approved these changes Oct 19, 2023

View reviewed changes

Mrtenz merged commit 0d68084 into main Oct 19, 2023
16 checks passed

Mrtenz deleted the mrtenz/json-object-validation branch October 19, 2023 20:20

legobeat mentioned this pull request Oct 23, 2023

Bump @metamask/utils from 8.1.0 to 8.2.0 MetaMask/eth-block-tracker#193

Closed

mcmire mentioned this pull request Nov 9, 2023

Bump @metamask/utils to ^8.2.1 MetaMask/rpc-errors#122

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add struct utils for validating JSON objects with optional values #136

Add struct utils for validating JSON objects with optional values #136

Mrtenz commented Aug 31, 2023 •

edited

Loading

This comment was marked as resolved.

legobeat commented Aug 31, 2023 •

edited by mcmire

Loading

danroc Sep 1, 2023

Mrtenz Sep 5, 2023

legobeat commented Sep 6, 2023

mcmire left a comment

mcmire Oct 19, 2023

Mrtenz Oct 19, 2023

mcmire Oct 19, 2023

mcmire Oct 19, 2023

Mrtenz Oct 19, 2023

mcmire Oct 19, 2023

Mrtenz Oct 19, 2023

mcmire Oct 19, 2023

Mrtenz Oct 19, 2023

mcmire left a comment

mcmire left a comment

Add struct utils for validating JSON objects with optional values #136

Add struct utils for validating JSON objects with optional values #136

Conversation

Mrtenz commented Aug 31, 2023 • edited Loading

This comment was marked as resolved.

legobeat commented Aug 31, 2023 • edited by mcmire Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legobeat commented Sep 6, 2023

mcmire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire left a comment

Choose a reason for hiding this comment

mcmire left a comment

Choose a reason for hiding this comment

Mrtenz commented Aug 31, 2023 •

edited

Loading

legobeat commented Aug 31, 2023 •

edited by mcmire

Loading