Upgrade to TypeScript 3.2 and fix protocol types (#9)

corhere · web-flow · commit 96483e971536 · 2018-12-13T14:35:21.000-05:00
TypeScript 3.2 brings typed `Function.call`, which has revealed a deficiency in our JSON-RPC type definitions. The `params` argument for the `RequestHandler` and `NotificationHandler` callbacks was incompatible with `undefined` in its union, even though an incoming RPC request could have its `params` property omitted. Correct this mistake by including `undefined` in the `RPCParams` union type. Finish fixing #5 by also allowing `params` to be omitted for notifications. Fix the unit tests which did not catch this issue before.
diff --git a/package.json b/package.json
@@ -30,7 +30,7 @@
     "ts-jest": "^23.10.4",
     "tslint": "^5.9.1",
     "tslint-config-airbnb": "^5.11.1",
-    "typescript": "^3.1.6"
+    "typescript": "^3.2.2"
   },
   "dependencies": {
     "@types/error-subclass": "^2.2.0",
diff --git a/src/protocol.test.ts b/src/protocol.test.ts
@@ -9,216 +9,122 @@ declare class Object {
 
 describe('parse', () => {
   describe('a valid Request', () => {
-    const vectors: {
-      desc: string;
-      id: string | number;
-      method: string;
-      params?: object | any[];
-    }[] = [
-      {
-        desc: 'with a numeric id',
-        id: -7,
-        method: 'foo.doBar/baz',
-      },
-      {
-        desc: 'with a string id',
-        id: 'three',
-        method: 'hello',
-      },
-      {
-        desc: 'with params as empty array',
-        id: 55,
-        method: 'foo',
-        params: [],
-      },
-      {
-        desc: 'with params by-position',
+    it.each([
+      ['with a numeric id', { id: -7, method: 'foo.doBar/baz' }],
+      ['with a string id', { id: 'three', method: 'hello' }],
+      ['with params as empty array', { id: 55, method: 'foo', params: [] }],
+      ['with params by-position', {
         id: 42,
         method: 'bar',
         params: [3, 'three', { three: 3 }, ['four', 'five']],
-      },
-      {
-        desc: 'with params by-name',
+      }],
+      ['with params by-name', {
         id: 8,
         method: 'baz',
         params: { foo: 'bar', quux: 'yes', 3: 5 } as object,
-      },
-    ];
-    vectors.forEach((vector) => {
-      test(vector.desc, () => {
-        const { id, method, params } = vector;
-        expect(jrpc.parse({
-          id, method, params, jsonrpc: '2.0',
-        })).toEqual({ id, method, params, kind: 'request' });
-      });
+      }],
+    ])('%s', (_, vector) => {
+      const { id, method, params } = vector;
+      // We can't reassemble the message from the spread object as any
+      // properties that do not exist in the test vector would come into
+      // existence on the reassembled message, with a value of
+      // undefined. This would invalidate the tests as a property
+      // that exists with the value undefined cannot be represented in
+      // JSON.
+      expect(jrpc.parse({ ...vector, jsonrpc: '2.0' }))
+        .toEqual({ id, method, params, kind: 'request' });
     });
   });
 
   describe('a valid Notification', () => {
-    const vectors: {
-      desc: string;
-      method: string;
-      params?: object | any[];
-    }[] = [
-      {
-        desc: 'with no params',
-        method: 'notifoo',
-      },
-      {
-        desc: 'with params as empty array',
-        method: 'blah',
-        params: [],
-      },
-      {
-        desc: 'with params by-position',
+    it.each([
+      ['with no params', { method: 'notifoo' }],
+      ['with params as empty array', { method: 'blah', params: [] }],
+      ['with params by-position', {
         method: 'a.method',
         params: [
-                    ['hello', 'bonjour', -7.2],
+          ['hello', 'bonjour', -7.2],
           'abc',
-                    { a: 3, b: 'four' },
+          { a: 3, b: 'four' },
         ],
-      },
-      {
-        desc: 'with params by-name',
+      }],
+      ['with params by-name', {
         method: 'yo',
         params: {
           a: 1,
           b: 'two',
           c: [3, 'four', 5],
           d: { e: 'f' },
-        } as object,
-      },
-    ];
-    vectors.forEach((vector) => {
-      test(vector.desc, () => {
-        const { method, params } = vector;
-        expect(jrpc.parse({
-          method, params, jsonrpc: '2.0',
-        })).toEqual({ method, params, kind: 'notification' });
-      });
+        },
+      }],
+    ])('%s', (_, vector) => {
+      const { method, params } = vector;
+      expect(jrpc.parse({ ...vector, jsonrpc: '2.0' }))
+        .toEqual({ method, params, kind: 'notification' });
     });
   });
 
   describe('a valid Response', () => {
-    const vectors: {
-      desc: string;
-      id: string | number;
-      result: any;
-    }[] = [
-      {
-        desc: 'with numeric id',
-        id: -7,
-        result: null,
-      },
-      {
-        desc: 'with string id',
-        id: 'abc',
-        result: null,
-      },
-      {
-        desc: 'with numeric result',
-        id: 5,
-        result: 3.7,
-      },
-      {
-        desc: 'with string result',
-        id: 48,
-        result: 'hello',
-      },
-      {
-        desc: 'with boolean result',
-        id: 1,
-        result: true,
-      },
-      {
-        desc: 'with array result',
+    it.each([
+      ['with numeric id', { id: -7, result: null }],
+      ['with string id', { id: 'abc', result: null }],
+      ['with numeric result', { id: 5, result: 3.7 }],
+      ['with string result', { id: 48, result: 'hello' }],
+      ['with boolean result', { id: 1, result: true }],
+      ['with array result', {
         id: 86,
         result: ['one', 2, { three: 3 }, [4]],
-      },
-      {
-        desc: 'with object result',
-        id: 104,
-        result: { yes: 'yup' },
-      },
-    ];
-    vectors.forEach((vector) => {
-      test(vector.desc, () => {
-        const { id, result } = vector;
-        expect(jrpc.parse({
-          id, result, jsonrpc: '2.0',
-        })).toEqual({
-          id, result, kind: 'response',
-        });
-      });
+      }],
+      ['with object result', { id: 104, result: { yes: 'yup' } }],
+    ])('%s', (_, vector) => {
+      const { id, result } = vector;
+      expect(jrpc.parse({ ...vector, jsonrpc: '2.0' }))
+        .toEqual({ id, result, kind: 'response' });
     });
   });
 
   describe('a valid Error', () => {
-    const vectors: {
-      desc: string;
-      id: string | number | null;
-      error: {
-        code: number;
-        message: string;
-        data?: any;
-      };
-    }[] = [
-      {
-        desc: 'with no id or data',
+    it.each([
+      ['with no id or data', {
         id: null,
         error: { code: -37000, message: 'you dun goofed' },
-      },
-      {
-        desc: 'with numeric id, no data',
+      }],
+      ['with numeric id, no data', {
         id: 7,
         error: { code: 42, message: 'everything' },
-      },
-      {
-        desc: 'with string id, no data',
+      }],
+      ['with string id, no data', {
         id: 'asdf',
         error: { code: 0, message: 'zero' },
-      },
-      {
-        desc: 'with boolean data',
+      }],
+      ['with boolean data', {
         id: 2,
         error: { code: 33, message: 'm', data: false },
-      },
-      {
-        desc: 'with numeric data',
+      }],
+      ['with numeric data', {
         id: 3,
         error: { code: 34, message: 'm', data: 8 },
-      },
-      {
-        desc: 'with string data',
+      }],
+      ['with string data', {
         id: 4,
         error: { code: 35, message: 'q', data: 'nope' },
-      },
-      {
-        desc: 'with null data',
+      }],
+      ['with null data', {
         id: 88,
         error: { code: 123, message: 'yes', data: null },
-      },
-      {
-        desc: 'with array data',
+      }],
+      ['with array data', {
         id: 5,
         error: { code: 36, message: 'r', data: [1, 2, 'three'] },
-      },
-      {
-        desc: 'with object data',
+      }],
+      ['with object data', {
         id: 6,
         error: { code: 37, message: 's', data: { foo: 'bar' } },
-      },
-    ];
-
-    vectors.forEach((vector) => {
-      test(vector.desc, () => {
-        const { id, error } = vector;
-        expect(jrpc.parse({
-          id, error, jsonrpc: '2.0',
-        })).toEqual({
-          id, error, kind: 'error',
-        });
-      });
+      }],
+    ])('%s', (_, vector) => {
+      const { id, error } = vector;
+      expect(jrpc.parse({ ...vector, jsonrpc: '2.0' }))
+        .toEqual({ id, error, kind: 'error' });
     });
   });
 
diff --git a/src/protocol.ts b/src/protocol.ts
@@ -30,7 +30,7 @@ export const RPCID = t.union([t.Integer, t.string]);
 export type RPCID = t.TypeOf<typeof RPCID>;
 
 /** JSON-RPC Request params */
-export const RPCParams = Structured;
+export const RPCParams = t.union([Structured, t.undefined]);
 export type RPCParams = t.TypeOf<typeof RPCParams>;
 
 /** `error` member of a JSON-RPC Error object */
@@ -65,9 +65,9 @@ export const NotificationJSON = t.intersection([
   t.interface({
     jsonrpc: t.literal('2.0'),
     method: t.string,
-    params: t.union([RPCParams, t.undefined]),
   }),
   t.partial({
+    params: RPCParams,
     id: t.undefined,
     result: t.undefined,
     error: t.undefined,
diff --git a/yarn.lock b/yarn.lock
@@ -3091,9 +3091,10 @@ type-check@~0.3.2:
   dependencies:
     prelude-ls "~1.1.2"
 
-typescript@^3.1.6:
-  version "3.1.6"
-  resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.1.6.tgz#b6543a83cfc8c2befb3f4c8fba6896f5b0c9be68"
+typescript@^3.2.2:
+  version "3.2.2"
+  resolved "https://registry.npmjs.org/typescript/-/typescript-3.2.2.tgz#fe8101c46aa123f8353523ebdcf5730c2ae493e5"
+  integrity sha512-VCj5UiSyHBjwfYacmDuc/NOk4QQixbE+Wn7MFJuS0nRuPQbof132Pw4u53dm264O8LPc2MVsc7RJNml5szurkg==
 
 uglify-js@^3.1.4:
   version "3.4.9"
