From 6c8390ee19eafec86f6499139fe253258f481f18 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Thu, 9 Apr 2020 08:27:16 +1000 Subject: [PATCH 1/9] add: promisify --- .../src/modifiers/__test__/promisify-test.js | 171 ++++++++++++++++++ .../dataparcels/src/modifiers/promisify.js | 60 ++++++ 2 files changed, 231 insertions(+) create mode 100644 packages/dataparcels/src/modifiers/__test__/promisify-test.js create mode 100644 packages/dataparcels/src/modifiers/promisify.js diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js new file mode 100644 index 00000000..204cb239 --- /dev/null +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -0,0 +1,171 @@ +// @flow +import promisify from '../promisify'; +import Parcel from '../../parcel/Parcel'; + +describe('promisify', () => { + it('should fire promise and resolve', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: 123, + handleChange + }); + + let promisifiedParcel = parcel + .modifyUp(promisify('foo', () => Promise.resolve())); + + expect(promisifiedParcel.value).toBe(123); + expect(promisifiedParcel.meta.fooStatus).toBe(undefined); + expect(promisifiedParcel.meta.fooError).toBe(undefined); + + promisifiedParcel.set(456); + + expect(handleChange).toHaveBeenCalledTimes(1); + + let newParcel = handleChange.mock.calls[0][0]; + expect(newParcel.value).toBe(456); + expect(newParcel.meta.fooStatus).toBe('pending'); + expect(newParcel.meta.fooError).toBe(undefined); + + await Promise.resolve(); + + expect(handleChange).toHaveBeenCalledTimes(2); + expect(handleChange.mock.calls[1][0].value).toBe(456); + expect(handleChange.mock.calls[1][0].meta.fooStatus).toBe('resolved'); + expect(handleChange.mock.calls[1][0].meta.fooError).toBe(undefined); + + window.setTimeout = realSetTimeout; + }); + + it('should fire promise and resolve with updated data', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: 123, + handleChange + }); + + parcel + .modifyUp(promisify('foo', ({value}) => Promise.resolve({ + value: value + 1 + }))) + .set(456); + + await Promise.resolve(); + + expect(handleChange).toHaveBeenCalledTimes(2); + expect(handleChange.mock.calls[1][0].value).toBe(457); + expect(handleChange.mock.calls[1][0].meta.fooStatus).toBe('resolved'); + expect(handleChange.mock.calls[1][0].meta.fooError).toBe(undefined); + + window.setTimeout = realSetTimeout; + }); + + it('should fire promise and reject', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: 123, + handleChange + }); + + parcel + .modifyUp(promisify('foo', () => Promise.reject('error!'))) + .set(456); + + await Promise.resolve(); + + expect(handleChange).toHaveBeenCalledTimes(2); + expect(handleChange.mock.calls[1][0].value).toBe(456); + expect(handleChange.mock.calls[1][0].meta.fooStatus).toBe('rejected'); + expect(handleChange.mock.calls[1][0].meta.fooError).toBe('error!'); + + window.setTimeout = realSetTimeout; + }); + + it('should only allow update from most recently fired promise to enforce order', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: '', + handleChange + }); + + let resolveFirstPromise = () => {}; + let firstPromise = new Promise(resolve => { + resolveFirstPromise = () => resolve({ + value: 'first-resolved' + }); + }); + + let resolveSecondPromise = () => {}; + let secondPromise = new Promise((resolve, reject) => { + resolveSecondPromise = () => reject('second-rejected'); + }); + + let promiseUpdater = promisify('foo', ({value}) => { + if(value === 'first') { + return firstPromise; + } + if(value === 'second') { + return secondPromise; + } + return Promise.resolve({ + value: 'third-resolved' + }); + }); + + parcel + .modifyUp(promiseUpdater) + .set('first'); + + handleChange.mock.calls[0][0] + .modifyUp(promiseUpdater) + .set('second'); + + handleChange.mock.calls[1][0] + .modifyUp(promiseUpdater) + .set('third'); + + expect(handleChange).toHaveBeenCalledTimes(3); + + await Promise.resolve(); + + expect(handleChange).toHaveBeenCalledTimes(4); + expect(handleChange.mock.calls[3][0].value).toBe('third-resolved'); + + resolveFirstPromise(); + await firstPromise; + + resolveSecondPromise(); + await secondPromise.catch(() => {}); + + expect(handleChange).toHaveBeenCalledTimes(4); + + window.setTimeout = realSetTimeout; + }); +}); diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js new file mode 100644 index 00000000..7cfd8c07 --- /dev/null +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -0,0 +1,60 @@ +// @flow + +type Data = { + value: any, + meta: {[key: string]: any} +}; + +type PartialData = { + value?: any, + meta?: {[key: string]: any} +}; + +type PromiseFunction = (data: Data) => Promise; +type Update = (updater: (data: Data) => PartialData) => void; + +export default (key: string, fn: PromiseFunction) => { + let count = 0; + + return (data: Data) => { + + let statusKey = `${key}Status`; + let errorKey = `${key}Error`; + let countAtCall = ++count; + + let meta = { + [statusKey]: 'pending', + [errorKey]: undefined + }; + + let effect = (update: Update) => fn(data).then( + (data) => { + if(count !== countAtCall) return; + data = data || {}; + update(() => ({ + ...data, + meta: { + // $FlowFixMe - this inference is wrong, data cannot be undefined here + ...(data.meta || {}), + [statusKey]: 'resolved', + [errorKey]: undefined + } + })); + }, + (error) => { + if(count !== countAtCall) return; + update(() => ({ + meta: { + [statusKey]: 'rejected', + [errorKey]: error + } + })); + } + ); + + return { + meta, + effect + }; + }; +}; From 95c0d51000f3057d74b5938e3a42b828fb409fe9 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Thu, 9 Apr 2020 08:32:20 +1000 Subject: [PATCH 2/9] add: export promisify --- packages/dataparcels/.size-limit.json | 4 ++++ packages/dataparcels/__test__/Exports-test.js | 6 ++++++ packages/dataparcels/package.json | 1 + packages/dataparcels/promisify.js | 2 ++ packages/react-dataparcels/.size-limit.json | 4 ++++ packages/react-dataparcels/__test__/Exports-test.js | 6 ++++++ packages/react-dataparcels/package.json | 1 + packages/react-dataparcels/promisify.js | 2 ++ 8 files changed, 26 insertions(+) create mode 100644 packages/dataparcels/promisify.js create mode 100644 packages/react-dataparcels/promisify.js diff --git a/packages/dataparcels/.size-limit.json b/packages/dataparcels/.size-limit.json index abed0bba..3bced28a 100644 --- a/packages/dataparcels/.size-limit.json +++ b/packages/dataparcels/.size-limit.json @@ -31,6 +31,10 @@ "limit": "5.9 KB", "path": "arrange.js" }, + { + "limit": "0.5 KB", + "path": "promisify.js" + }, { "limit": "2.2 KB", "path": "translate.js" diff --git a/packages/dataparcels/__test__/Exports-test.js b/packages/dataparcels/__test__/Exports-test.js index ea2b56ef..c8218877 100644 --- a/packages/dataparcels/__test__/Exports-test.js +++ b/packages/dataparcels/__test__/Exports-test.js @@ -9,6 +9,7 @@ import deleted from '../deleted'; import ParcelNode from '../ParcelNode'; import arrange from '../arrange'; import cancel from '../cancel'; +import promisify from '../promisify'; import translate from '../translate'; import validation from '../validation'; @@ -23,6 +24,7 @@ import Internaldeleted from '../lib/parcelData/deleted'; import InternalParcelNode from '../lib/parcelNode/ParcelNode'; import InternalAsNodes from '../lib/parcelNode/arrange'; import Internalcancel from '../lib/change/cancel'; +import InternalPromisify from '../lib/modifiers/promisify'; import InternalTranslate from '../lib/modifiers/translate'; import InternalValidation from '../lib/validation/validation'; @@ -58,6 +60,10 @@ test('/cancel should export cancel', () => { expect(cancel).toBe(Internalcancel); }); +test('/promisify should export promisify', () => { + expect(promisify).toBe(InternalPromisify); +}); + test('/translate should export translate', () => { expect(translate).toBe(InternalTranslate); }); diff --git a/packages/dataparcels/package.json b/packages/dataparcels/package.json index 8dc11b9c..e2a4af4b 100644 --- a/packages/dataparcels/package.json +++ b/packages/dataparcels/package.json @@ -18,6 +18,7 @@ "combine.js", "deleted.js", "ParcelNode.js", + "promisify.js", "translate.js", "validation.js" ], diff --git a/packages/dataparcels/promisify.js b/packages/dataparcels/promisify.js new file mode 100644 index 00000000..b1f880b8 --- /dev/null +++ b/packages/dataparcels/promisify.js @@ -0,0 +1,2 @@ +/* eslint-disable */ +module.exports = require('./lib/modifiers/promisify.js').default; diff --git a/packages/react-dataparcels/.size-limit.json b/packages/react-dataparcels/.size-limit.json index c54c02d2..b959870c 100644 --- a/packages/react-dataparcels/.size-limit.json +++ b/packages/react-dataparcels/.size-limit.json @@ -31,6 +31,10 @@ "limit": "5.9 KB", "path": "arrange.js" }, + { + "limit": "0.5 KB", + "path": "promisify.js" + }, { "limit": "6.1 KB", "path": "translate.js" diff --git a/packages/react-dataparcels/__test__/Exports-test.js b/packages/react-dataparcels/__test__/Exports-test.js index ce71b116..f71c632f 100644 --- a/packages/react-dataparcels/__test__/Exports-test.js +++ b/packages/react-dataparcels/__test__/Exports-test.js @@ -9,6 +9,7 @@ import deleted from '../deleted'; import ParcelNode from '../ParcelNode'; import arrange from '../arrange'; import cancel from '../cancel'; +import promisify from '../promisify'; import translate from '../translate'; import validation from '../validation'; @@ -33,6 +34,7 @@ import Internaldeleted from 'dataparcels/deleted'; import InternalParcelNode from 'dataparcels/ParcelNode'; import InternalAsNodes from 'dataparcels/arrange'; import Internalcancel from 'dataparcels/cancel'; +import InternalPromisify from 'dataparcels/promisify'; import InternalTranslate from 'dataparcels/translate'; import InternalValidation from 'dataparcels/validation'; @@ -80,6 +82,10 @@ test('/cancel should export cancel', () => { expect(cancel).toBe(Internalcancel); }); +test('/promisify should export promisify', () => { + expect(promisify).toBe(InternalPromisify); +}); + test('/translate should export translate', () => { expect(translate).toBe(InternalTranslate); }); diff --git a/packages/react-dataparcels/package.json b/packages/react-dataparcels/package.json index 128295a1..2172e243 100644 --- a/packages/react-dataparcels/package.json +++ b/packages/react-dataparcels/package.json @@ -30,6 +30,7 @@ "useParcelBuffer.js", "useParcelForm.js", "useParcelState.js", + "promisify.js", "translate.js", "validation.js" ], diff --git a/packages/react-dataparcels/promisify.js b/packages/react-dataparcels/promisify.js new file mode 100644 index 00000000..c91d9c2c --- /dev/null +++ b/packages/react-dataparcels/promisify.js @@ -0,0 +1,2 @@ +/* eslint-disable */ +module.exports = require('dataparcels/promisify.js'); From 9c8ae3867b7bf1ba56bd7e2471f49a619cade2b2 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Thu, 9 Apr 2020 08:32:52 +1000 Subject: [PATCH 3/9] docs: update dev page --- packages/dataparcels-docs/src/pages/dev.js | 97 ++++++++++++++-------- 1 file changed, 63 insertions(+), 34 deletions(-) diff --git a/packages/dataparcels-docs/src/pages/dev.js b/packages/dataparcels-docs/src/pages/dev.js index fa69cf23..e506b37b 100644 --- a/packages/dataparcels-docs/src/pages/dev.js +++ b/packages/dataparcels-docs/src/pages/dev.js @@ -1,41 +1,53 @@ // @flow import React from 'react'; +import {useRef} from 'react'; import Page from 'component/Page'; import {H1} from 'dcme-style'; import {ContentNav} from 'dcme-style'; -import useParcelState from 'react-dataparcels/useParcelState'; -import ParcelBoundary from 'react-dataparcels/ParcelBoundary'; +import promisify from 'react-dataparcels/promisify'; +import useParcel from 'react-dataparcels/useParcel'; +import Boundary from 'react-dataparcels/Boundary'; export default function PersonEditor() { - let [personParcel] = useParcelState({ - value: { - firstname: "Robert", - lastname: "Clamps" - } - }); + let rejectRef = useRef(); - personParcel = personParcel - .modifyUp(({changeRequest}) => { - // cause the change request reducer to re-execute the same effect again - changeRequest.nextData; - }) - .modifyUp(({value}) => { - let {firstname} = value; + let personParcel = useParcel({ + // source: () => ({ + // value: { + // firstname: "Robert", + // lastname: "Clamps", + // saves: 0 + // } + // }), + source: promisify('load', async () => { + await new Promise(resolve => setTimeout(resolve, 1000)); return { - effect: async (update) => { - await new Promise(resolve => setTimeout(resolve, firstname.length > 2 ? 0 : 1000)); - - update(({value}) => ({ - value: { - ...value, - lastname: firstname - } - })); + value: { + firstname: "Robert", + lastname: "Clamps", + saves: 0 } }; - }); + }), + onChange: promisify('save', async ({value}) => { + await new Promise(resolve => setTimeout(resolve, 1000)); + + if(rejectRef.current) { + rejectRef.current = false; + throw new Error('NO!!!'); + } + + return { + value: { + ...value, + saves: value.saves + 1 + } + }; + }), + buffer: true + }); return

Dev page

- - - {(firstname) => } - - - - - {(lastname) => } - +
load status {personParcel.meta.loadStatus}
+ + {personParcel.value && + <> +
firstname
+ + {(firstname) => } + + +
lastname
+ + {(lastname) => } + + +
save status {personParcel.meta.saveStatus}
+
save error {personParcel.meta.saveError && personParcel.meta.saveError.message}
+
saves {personParcel.value.saves}
+ +
+ +
+
+ +
+ + } ; } From a95dca34b77705b8f7732adf7c0c92a8acb45371 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Fri, 10 Apr 2020 11:49:47 +1000 Subject: [PATCH 4/9] amend: promisify api to be less breakable --- packages/dataparcels-docs/src/pages/dev.js | 49 +++++++++++-------- .../src/modifiers/__test__/promisify-test.js | 40 +++++++++------ .../dataparcels/src/modifiers/promisify.js | 9 +++- 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/packages/dataparcels-docs/src/pages/dev.js b/packages/dataparcels-docs/src/pages/dev.js index e506b37b..f023da97 100644 --- a/packages/dataparcels-docs/src/pages/dev.js +++ b/packages/dataparcels-docs/src/pages/dev.js @@ -21,30 +21,37 @@ export default function PersonEditor() { // saves: 0 // } // }), - source: promisify('load', async () => { - await new Promise(resolve => setTimeout(resolve, 1000)); - return { - value: { - firstname: "Robert", - lastname: "Clamps", - saves: 0 - } - }; - }), - onChange: promisify('save', async ({value}) => { - await new Promise(resolve => setTimeout(resolve, 1000)); - - if(rejectRef.current) { - rejectRef.current = false; - throw new Error('NO!!!'); + source: promisify({ + key: 'load', + effect: async () => { + await new Promise(resolve => setTimeout(resolve, 1000)); + return { + value: { + firstname: "Robert", + lastname: "Clamps", + saves: 0 + } + }; } + }), + onChange: promisify({ + key: 'save', + effect: async ({value}) => { + await new Promise(resolve => setTimeout(resolve, 1000)); - return { - value: { - ...value, - saves: value.saves + 1 + if(rejectRef.current) { + rejectRef.current = false; + throw new Error('NO!!!'); } - }; + + return { + value: { + ...value, + saves: value.saves + 1 + } + }; + }, + revert: true }), buffer: true }); diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js index 204cb239..51c241f6 100644 --- a/packages/dataparcels/src/modifiers/__test__/promisify-test.js +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -18,7 +18,10 @@ describe('promisify', () => { }); let promisifiedParcel = parcel - .modifyUp(promisify('foo', () => Promise.resolve())); + .modifyUp(promisify({ + key: 'foo', + effect: () => Promise.resolve() + })); expect(promisifiedParcel.value).toBe(123); expect(promisifiedParcel.meta.fooStatus).toBe(undefined); @@ -58,9 +61,12 @@ describe('promisify', () => { }); parcel - .modifyUp(promisify('foo', ({value}) => Promise.resolve({ - value: value + 1 - }))) + .modifyUp(promisify({ + key: 'foo', + effect: ({value}) => Promise.resolve({ + value: value + 1 + }) + })) .set(456); await Promise.resolve(); @@ -88,7 +94,10 @@ describe('promisify', () => { }); parcel - .modifyUp(promisify('foo', () => Promise.reject('error!'))) + .modifyUp(promisify({ + key: 'foo', + effect: () => Promise.reject('error!') + })) .set(456); await Promise.resolve(); @@ -127,16 +136,19 @@ describe('promisify', () => { resolveSecondPromise = () => reject('second-rejected'); }); - let promiseUpdater = promisify('foo', ({value}) => { - if(value === 'first') { - return firstPromise; + let promiseUpdater = promisify({ + key: 'foo', + effect: ({value}) => { + if(value === 'first') { + return firstPromise; + } + if(value === 'second') { + return secondPromise; + } + return Promise.resolve({ + value: 'third-resolved' + }); } - if(value === 'second') { - return secondPromise; - } - return Promise.resolve({ - value: 'third-resolved' - }); }); parcel diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js index 7cfd8c07..689a02c1 100644 --- a/packages/dataparcels/src/modifiers/promisify.js +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -13,7 +13,14 @@ type PartialData = { type PromiseFunction = (data: Data) => Promise; type Update = (updater: (data: Data) => PartialData) => void; -export default (key: string, fn: PromiseFunction) => { +type Config = { + key: string, + effect: PromiseFunction +}; + +export default (config: Config) => { + let {key} = config; + let fn = config.effect; let count = 0; return (data: Data) => { From cce4527b4b9a0aaaf55a6924dd085f25a55697dc Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Fri, 10 Apr 2020 12:05:49 +1000 Subject: [PATCH 5/9] add: allow porisify to return updater function --- packages/dataparcels/.size-limit.json | 2 +- .../src/modifiers/__test__/promisify-test.js | 44 +++++++++++++++++++ .../dataparcels/src/modifiers/promisify.js | 28 ++++++------ packages/react-dataparcels/.size-limit.json | 2 +- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/packages/dataparcels/.size-limit.json b/packages/dataparcels/.size-limit.json index 3bced28a..418e5dd4 100644 --- a/packages/dataparcels/.size-limit.json +++ b/packages/dataparcels/.size-limit.json @@ -32,7 +32,7 @@ "path": "arrange.js" }, { - "limit": "0.5 KB", + "limit": "0.6 KB", "path": "promisify.js" }, { diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js index 51c241f6..e8bf03c5 100644 --- a/packages/dataparcels/src/modifiers/__test__/promisify-test.js +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -79,6 +79,50 @@ describe('promisify', () => { window.setTimeout = realSetTimeout; }); + it('should fire promise and resolve using an updater function', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + let updater = jest.fn(({value}) => ({ + value: value + 1 + })); + + let parcel = new Parcel({ + value: 123, + handleChange + }); + + let resolvePromise = () => {}; + let promise = new Promise(resolve => { + resolvePromise = () => resolve(updater); + }); + + parcel + .modifyUp(promisify({ + key: 'foo', + effect: ({value}) => promise + })) + .set(456); + + parcel + .set(700); + + resolvePromise(); + await promise; + + expect(handleChange).toHaveBeenCalledTimes(3); + + expect(updater).toHaveBeenCalledTimes(1); + expect(updater.mock.calls[0][0].value).toBe(700); + expect(handleChange.mock.calls[2][0].value).toBe(701); + + window.setTimeout = realSetTimeout; + }); + it('should fire promise and reject', async () => { // remove setTimeout because jest doesnt handle diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js index 689a02c1..e106fa10 100644 --- a/packages/dataparcels/src/modifiers/promisify.js +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -1,4 +1,5 @@ // @flow +import combine from '../parcelData/combine'; type Data = { value: any, @@ -10,8 +11,9 @@ type PartialData = { meta?: {[key: string]: any} }; -type PromiseFunction = (data: Data) => Promise; -type Update = (updater: (data: Data) => PartialData) => void; +type Updater = (data: Data) => PartialData; +type Update = (updater: Updater) => void; +type PromiseFunction = (data: Data) => Promise; type Config = { key: string, @@ -35,18 +37,18 @@ export default (config: Config) => { }; let effect = (update: Update) => fn(data).then( - (data) => { + (result) => { if(count !== countAtCall) return; - data = data || {}; - update(() => ({ - ...data, - meta: { - // $FlowFixMe - this inference is wrong, data cannot be undefined here - ...(data.meta || {}), - [statusKey]: 'resolved', - [errorKey]: undefined - } - })); + + update(combine( + typeof result !== 'function' ? () => result : result, + () => ({ + meta: { + [statusKey]: 'resolved', + [errorKey]: undefined + } + }) + )); }, (error) => { if(count !== countAtCall) return; diff --git a/packages/react-dataparcels/.size-limit.json b/packages/react-dataparcels/.size-limit.json index b959870c..fe8d69db 100644 --- a/packages/react-dataparcels/.size-limit.json +++ b/packages/react-dataparcels/.size-limit.json @@ -32,7 +32,7 @@ "path": "arrange.js" }, { - "limit": "0.5 KB", + "limit": "0.6 KB", "path": "promisify.js" }, { From 81579d4929636fc6e0e9cc92604befd3cf125baa Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Fri, 10 Apr 2020 12:11:00 +1000 Subject: [PATCH 6/9] add: promisify revert to cope with failed saves etc --- .../src/modifiers/__test__/promisify-test.js | 32 +++++++++++++++++++ .../dataparcels/src/modifiers/promisify.js | 26 +++++++++------ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js index e8bf03c5..756529e5 100644 --- a/packages/dataparcels/src/modifiers/__test__/promisify-test.js +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -154,6 +154,38 @@ describe('promisify', () => { window.setTimeout = realSetTimeout; }); + it('should fire promise and reject and revert', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: 123, + handleChange + }); + + parcel + .modifyUp(promisify({ + key: 'foo', + effect: () => Promise.reject('error!'), + revert: true + })) + .set(456); + + await Promise.resolve(); + + expect(handleChange).toHaveBeenCalledTimes(2); + expect(handleChange.mock.calls[1][0].value).toBe(123); + expect(handleChange.mock.calls[1][0].meta.fooStatus).toBe('rejected'); + expect(handleChange.mock.calls[1][0].meta.fooError).toBe('error!'); + + window.setTimeout = realSetTimeout; + }); + it('should only allow update from most recently fired promise to enforce order', async () => { // remove setTimeout because jest doesnt handle diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js index e106fa10..1abda277 100644 --- a/packages/dataparcels/src/modifiers/promisify.js +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -1,9 +1,12 @@ // @flow +import type ChangeRequest from '../change/ChangeRequest'; + import combine from '../parcelData/combine'; type Data = { value: any, - meta: {[key: string]: any} + meta: {[key: string]: any}, + changeRequest: ChangeRequest }; type PartialData = { @@ -17,11 +20,12 @@ type PromiseFunction = (data: Data) => Promise; type Config = { key: string, - effect: PromiseFunction + effect: PromiseFunction, + revert?: boolean }; export default (config: Config) => { - let {key} = config; + let {key, revert} = config; let fn = config.effect; let count = 0; @@ -39,7 +43,6 @@ export default (config: Config) => { let effect = (update: Update) => fn(data).then( (result) => { if(count !== countAtCall) return; - update(combine( typeof result !== 'function' ? () => result : result, () => ({ @@ -52,12 +55,15 @@ export default (config: Config) => { }, (error) => { if(count !== countAtCall) return; - update(() => ({ - meta: { - [statusKey]: 'rejected', - [errorKey]: error - } - })); + update(combine( + () => revert ? data.changeRequest.prevData : {}, + () => ({ + meta: { + [statusKey]: 'rejected', + [errorKey]: error + } + }) + )); } ); From 160af08edadab0b5032959a73d55e00394228b41 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Sat, 11 Apr 2020 10:32:55 +1000 Subject: [PATCH 7/9] amend: enforce order always, make only taking last optional --- .../src/modifiers/__test__/promisify-test.js | 123 ++++++++++++++++-- .../dataparcels/src/modifiers/promisify.js | 65 +++++---- 2 files changed, 148 insertions(+), 40 deletions(-) diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js index 756529e5..734f1460 100644 --- a/packages/dataparcels/src/modifiers/__test__/promisify-test.js +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -2,6 +2,17 @@ import promisify from '../promisify'; import Parcel from '../../parcel/Parcel'; +let allResolvedPromises = async () => { + // for jest to await Promise.resolve() that are not explicitly returned + // you need to call a await Promise.resolve(); for each Promise.resolve() + // these tests shouldnt have to care how many internl Promise.resolve()s there are + // so just call it heaps + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); +}; + describe('promisify', () => { it('should fire promise and resolve', async () => { @@ -36,7 +47,7 @@ describe('promisify', () => { expect(newParcel.meta.fooStatus).toBe('pending'); expect(newParcel.meta.fooError).toBe(undefined); - await Promise.resolve(); + await allResolvedPromises(); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange.mock.calls[1][0].value).toBe(456); @@ -69,7 +80,7 @@ describe('promisify', () => { })) .set(456); - await Promise.resolve(); + await allResolvedPromises(); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange.mock.calls[1][0].value).toBe(457); @@ -114,6 +125,8 @@ describe('promisify', () => { resolvePromise(); await promise; + await allResolvedPromises(); + expect(handleChange).toHaveBeenCalledTimes(3); expect(updater).toHaveBeenCalledTimes(1); @@ -144,7 +157,7 @@ describe('promisify', () => { })) .set(456); - await Promise.resolve(); + await allResolvedPromises(); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange.mock.calls[1][0].value).toBe(456); @@ -176,7 +189,7 @@ describe('promisify', () => { })) .set(456); - await Promise.resolve(); + await allResolvedPromises(); expect(handleChange).toHaveBeenCalledTimes(2); expect(handleChange.mock.calls[1][0].value).toBe(123); @@ -186,7 +199,87 @@ describe('promisify', () => { window.setTimeout = realSetTimeout; }); - it('should only allow update from most recently fired promise to enforce order', async () => { + it('should process results in the same order they were fired', async () => { + + // remove setTimeout because jest doesnt handle + // setTimeouts and promises all mixed together like this + let realSetTimeout = window.setTimeout; + window.setTimeout = (fn, ms) => fn(); + + let handleChange = jest.fn(); + + let parcel = new Parcel({ + value: '', + handleChange + }); + + let resolveFirstPromise = () => {}; + let firstPromise = new Promise(resolve => { + resolveFirstPromise = () => resolve({ + value: 'first-resolved' + }); + }); + + let resolveSecondPromise = () => {}; + let secondPromise = new Promise(resolve => { + resolveSecondPromise = () => resolve({ + value: 'second-resolved' + }); + }); + + let resolveThirdPromise = () => {}; + let thirdPromise = new Promise(resolve => { + resolveThirdPromise = () => resolve({ + value: 'third-resolved' + }); + }); + + let promiseUpdater = promisify({ + key: 'foo', + effect: ({value}) => { + if(value === 'first') { + return firstPromise; + } + if(value === 'second') { + return secondPromise; + } + return thirdPromise; + } + }); + + parcel + .modifyUp(promiseUpdater) + .set('first'); + + handleChange.mock.calls[0][0] + .modifyUp(promiseUpdater) + .set('second'); + + handleChange.mock.calls[1][0] + .modifyUp(promiseUpdater) + .set('third'); + + expect(handleChange).toHaveBeenCalledTimes(3); + + resolveThirdPromise(); + await thirdPromise; + + resolveFirstPromise(); + await firstPromise; + + resolveSecondPromise(); + await secondPromise.catch(() => {}); + + // now that 3rd has a + expect(handleChange).toHaveBeenCalledTimes(6); + expect(handleChange.mock.calls[3][0].value).toBe('first-resolved'); + expect(handleChange.mock.calls[4][0].value).toBe('second-resolved'); + expect(handleChange.mock.calls[5][0].value).toBe('third-resolved'); + + window.setTimeout = realSetTimeout; + }); + + it('should only allow update from most recently fired promise if last = true', async () => { // remove setTimeout because jest doesnt handle // setTimeouts and promises all mixed together like this @@ -212,6 +305,13 @@ describe('promisify', () => { resolveSecondPromise = () => reject('second-rejected'); }); + let resolveThirdPromise = () => {}; + let thirdPromise = new Promise(resolve => { + resolveThirdPromise = () => resolve({ + value: 'third-resolved' + }); + }); + let promiseUpdater = promisify({ key: 'foo', effect: ({value}) => { @@ -221,10 +321,9 @@ describe('promisify', () => { if(value === 'second') { return secondPromise; } - return Promise.resolve({ - value: 'third-resolved' - }); - } + return thirdPromise; + }, + last: true }); parcel @@ -241,10 +340,9 @@ describe('promisify', () => { expect(handleChange).toHaveBeenCalledTimes(3); - await Promise.resolve(); - expect(handleChange).toHaveBeenCalledTimes(4); - expect(handleChange.mock.calls[3][0].value).toBe('third-resolved'); + resolveThirdPromise(); + await thirdPromise; resolveFirstPromise(); await firstPromise; @@ -253,6 +351,7 @@ describe('promisify', () => { await secondPromise.catch(() => {}); expect(handleChange).toHaveBeenCalledTimes(4); + expect(handleChange.mock.calls[3][0].value).toBe('third-resolved'); window.setTimeout = realSetTimeout; }); diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js index 1abda277..edbd3c34 100644 --- a/packages/dataparcels/src/modifiers/promisify.js +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -21,13 +21,15 @@ type PromiseFunction = (data: Data) => Promise; type Config = { key: string, effect: PromiseFunction, - revert?: boolean + revert?: boolean, + last?: boolean }; export default (config: Config) => { - let {key, revert} = config; + let {key, revert, last} = config; let fn = config.effect; let count = 0; + let chain = Promise.resolve(); return (data: Data) => { @@ -40,32 +42,39 @@ export default (config: Config) => { [errorKey]: undefined }; - let effect = (update: Update) => fn(data).then( - (result) => { - if(count !== countAtCall) return; - update(combine( - typeof result !== 'function' ? () => result : result, - () => ({ - meta: { - [statusKey]: 'resolved', - [errorKey]: undefined - } - }) - )); - }, - (error) => { - if(count !== countAtCall) return; - update(combine( - () => revert ? data.changeRequest.prevData : {}, - () => ({ - meta: { - [statusKey]: 'rejected', - [errorKey]: error - } - }) - )); - } - ); + let effect = (update: Update) => { + let lastChain = chain; + chain = fn(data).then( + (result) => { + if(last && count !== countAtCall) return; + lastChain.then(() => { + update(combine( + typeof result !== 'function' ? () => result : result, + () => ({ + meta: { + [statusKey]: 'resolved', + [errorKey]: undefined + } + }) + )); + }); + }, + (error) => { + if(last && count !== countAtCall) return; + lastChain.then(() => { + update(combine( + () => revert ? data.changeRequest.prevData : {}, + () => ({ + meta: { + [statusKey]: 'rejected', + [errorKey]: error + } + }) + )); + }); + } + ); + }; return { meta, From 54623a9352cfcffe7bf8ac288912f9b15af7f630 Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Sat, 11 Apr 2020 10:51:00 +1000 Subject: [PATCH 8/9] refactor: shrink promisify size --- .../src/modifiers/__test__/promisify-test.js | 8 +++- .../dataparcels/src/modifiers/promisify.js | 48 ++++++++----------- 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/packages/dataparcels/src/modifiers/__test__/promisify-test.js b/packages/dataparcels/src/modifiers/__test__/promisify-test.js index 734f1460..885cc96c 100644 --- a/packages/dataparcels/src/modifiers/__test__/promisify-test.js +++ b/packages/dataparcels/src/modifiers/__test__/promisify-test.js @@ -11,6 +11,9 @@ let allResolvedPromises = async () => { await Promise.resolve(); await Promise.resolve(); await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); }; describe('promisify', () => { @@ -270,6 +273,8 @@ describe('promisify', () => { resolveSecondPromise(); await secondPromise.catch(() => {}); + await allResolvedPromises(); + // now that 3rd has a expect(handleChange).toHaveBeenCalledTimes(6); expect(handleChange.mock.calls[3][0].value).toBe('first-resolved'); @@ -340,7 +345,6 @@ describe('promisify', () => { expect(handleChange).toHaveBeenCalledTimes(3); - resolveThirdPromise(); await thirdPromise; @@ -350,6 +354,8 @@ describe('promisify', () => { resolveSecondPromise(); await secondPromise.catch(() => {}); + await allResolvedPromises(); + expect(handleChange).toHaveBeenCalledTimes(4); expect(handleChange.mock.calls[3][0].value).toBe('third-resolved'); diff --git a/packages/dataparcels/src/modifiers/promisify.js b/packages/dataparcels/src/modifiers/promisify.js index edbd3c34..c3924cf0 100644 --- a/packages/dataparcels/src/modifiers/promisify.js +++ b/packages/dataparcels/src/modifiers/promisify.js @@ -44,36 +44,26 @@ export default (config: Config) => { let effect = (update: Update) => { let lastChain = chain; - chain = fn(data).then( - (result) => { + chain = fn(data) + .then(result => ({result}), error => ({error})) + .then(data => lastChain.then((): any => data)) + .then(({result, error}) => { if(last && count !== countAtCall) return; - lastChain.then(() => { - update(combine( - typeof result !== 'function' ? () => result : result, - () => ({ - meta: { - [statusKey]: 'resolved', - [errorKey]: undefined - } - }) - )); - }); - }, - (error) => { - if(last && count !== countAtCall) return; - lastChain.then(() => { - update(combine( - () => revert ? data.changeRequest.prevData : {}, - () => ({ - meta: { - [statusKey]: 'rejected', - [errorKey]: error - } - }) - )); - }); - } - ); + + if(error) { + result = revert ? data.changeRequest.prevData : {}; + } + + update(combine( + typeof result !== 'function' ? () => result : result, + () => ({ + meta: { + [statusKey]: error ? 'rejected' : 'resolved', + [errorKey]: error + } + }) + )); + }); }; return { From 20df131dfeeede54b74ea810f5cceb1d3f8536ff Mon Sep 17 00:00:00 2001 From: Damien Clarke Date: Sat, 11 Apr 2020 14:00:23 +1000 Subject: [PATCH 9/9] test: increase size limit --- packages/dataparcels/.size-limit.json | 2 +- packages/react-dataparcels/.size-limit.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dataparcels/.size-limit.json b/packages/dataparcels/.size-limit.json index 418e5dd4..495dd4db 100644 --- a/packages/dataparcels/.size-limit.json +++ b/packages/dataparcels/.size-limit.json @@ -32,7 +32,7 @@ "path": "arrange.js" }, { - "limit": "0.6 KB", + "limit": "0.7 KB", "path": "promisify.js" }, { diff --git a/packages/react-dataparcels/.size-limit.json b/packages/react-dataparcels/.size-limit.json index fe8d69db..c6baf790 100644 --- a/packages/react-dataparcels/.size-limit.json +++ b/packages/react-dataparcels/.size-limit.json @@ -32,7 +32,7 @@ "path": "arrange.js" }, { - "limit": "0.6 KB", + "limit": "0.7 KB", "path": "promisify.js" }, {