From 96483e971536e651098f0a296dff89007e91a7e0 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Thu, 13 Dec 2018 14:35:21 -0500 Subject: [PATCH] Upgrade to TypeScript 3.2 and fix protocol types (#9) 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. --- package.json | 2 +- src/protocol.test.ts | 230 +++++++++++++------------------------------ src/protocol.ts | 4 +- yarn.lock | 7 +- 4 files changed, 75 insertions(+), 168 deletions(-) diff --git a/package.json b/package.json index 0016043..a06c279 100644 --- 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 index 15c843f..11bc3aa 100644 --- 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 index 55ba889..b27feda 100644 --- 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; /** JSON-RPC Request params */ -export const RPCParams = Structured; +export const RPCParams = t.union([Structured, t.undefined]); export type RPCParams = t.TypeOf; /** `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 index 9d19719..dbbdd09 100644 --- 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"