From e5c7fd45ac5b09ca0d746ea09977536df1efb488 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 23 Mar 2020 14:28:34 +0100 Subject: [PATCH 1/4] fix: Make sure that SyncPromise handler is called only once --- packages/utils/src/syncpromise.ts | 37 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index 6a9f5e58944d..efcdb1b0f4b9 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -17,6 +17,7 @@ enum States { class SyncPromise implements PromiseLike { private _state: States = States.PENDING; private _handlers: Array<{ + done: boolean; onfulfilled?: ((value: T) => T | PromiseLike) | null; onrejected?: ((reason: any) => any) | null; }> = []; @@ -90,6 +91,7 @@ class SyncPromise implements PromiseLike { ): PromiseLike { return new SyncPromise((resolve, reject) => { this._attachHandler({ + done: false, onfulfilled: result => { if (!onfulfilled) { // TODO: ¯\_(ツ)_/¯ @@ -156,8 +158,7 @@ class SyncPromise implements PromiseLike { return; } - // tslint:disable-next-line:no-unsafe-any - resolve(val); + resolve((val as unknown) as any); }); }); } @@ -192,6 +193,8 @@ class SyncPromise implements PromiseLike { // TODO: FIXME /** JSDoc */ private readonly _attachHandler = (handler: { + /** JSDoc */ + done: boolean; /** JSDoc */ onfulfilled?(value: T): any; /** JSDoc */ @@ -207,22 +210,28 @@ class SyncPromise implements PromiseLike { return; } - if (this._state === States.REJECTED) { - this._handlers.forEach(handler => { + const cachedHandlers = this._handlers.slice(); + this._handlers = []; + + cachedHandlers.forEach(handler => { + if (handler.done) { + return; + } + + if (this._state === States.RESOLVED) { + if (handler.onfulfilled) { + handler.onfulfilled((this._value as unknown) as any); + } + } + + if (this._state === States.REJECTED) { if (handler.onrejected) { handler.onrejected(this._value); } - }); - } else { - this._handlers.forEach(handler => { - if (handler.onfulfilled) { - // tslint:disable-next-line:no-unsafe-any - handler.onfulfilled(this._value); - } - }); - } + } - this._handlers = []; + handler.done = true; + }); }; } From 55fed347fba77535365731e4f000efee963b3f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 23 Mar 2020 14:46:00 +0100 Subject: [PATCH 2/4] happy linter is happy --- packages/utils/src/supports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index 87a903fd6653..82ea8d378d80 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -106,7 +106,7 @@ export function supportsNativeFetch(): boolean { // so create a "pure" iframe to see if that has native fetch let result = false; const doc = global.document; - if (doc && typeof doc.createElement === 'function') { + if (doc && typeof (doc as object).createElement === 'function') { try { const sandbox = doc.createElement('iframe'); sandbox.hidden = true; From 93dc899f749ffb49ff2a1f24cd2cc20b2d2713e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 23 Mar 2020 15:35:21 +0100 Subject: [PATCH 3/4] linters plz --- packages/utils/src/supports.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/utils/src/supports.ts b/packages/utils/src/supports.ts index 82ea8d378d80..43e3a66a3059 100644 --- a/packages/utils/src/supports.ts +++ b/packages/utils/src/supports.ts @@ -106,7 +106,8 @@ export function supportsNativeFetch(): boolean { // so create a "pure" iframe to see if that has native fetch let result = false; const doc = global.document; - if (doc && typeof (doc as object).createElement === 'function') { + // tslint:disable-next-line:no-unbound-method deprecation + if (doc && typeof (doc.createElement as unknown) === `function`) { try { const sandbox = doc.createElement('iframe'); sandbox.hidden = true; From 0ffc01c5ec65ab235e3d9203d6056b79f81178f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Mon, 23 Mar 2020 15:40:01 +0100 Subject: [PATCH 4/4] Use SyncPromise inside Buffer tests --- packages/utils/test/promisebuffer.test.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/utils/test/promisebuffer.test.ts b/packages/utils/test/promisebuffer.test.ts index 26379e4fbc5b..64d0ef823da1 100644 --- a/packages/utils/test/promisebuffer.test.ts +++ b/packages/utils/test/promisebuffer.test.ts @@ -1,4 +1,5 @@ import { PromiseBuffer } from '../src/promisebuffer'; +import { SyncPromise } from '../src/syncpromise'; // tslint:disable:no-floating-promises @@ -10,15 +11,15 @@ describe('PromiseBuffer', () => { describe('add()', () => { test('no limit', () => { const q = new PromiseBuffer(); - const p = new Promise(resolve => setTimeout(resolve, 1)); + const p = new SyncPromise(resolve => setTimeout(resolve, 1)); q.add(p); expect(q.length()).toBe(1); }); test('with limit', () => { const q = new PromiseBuffer(1); - const p = new Promise(resolve => setTimeout(resolve, 1)); + const p = new SyncPromise(resolve => setTimeout(resolve, 1)); expect(q.add(p)).toEqual(p); - expect(q.add(new Promise(resolve => setTimeout(resolve, 1)))).rejects.toThrowError(); + expect(q.add(new SyncPromise(resolve => setTimeout(resolve, 1)))).rejects.toThrowError(); expect(q.length()).toBe(1); }); }); @@ -26,7 +27,7 @@ describe('PromiseBuffer', () => { test('resolved promises should not show up in buffer length', async () => { expect.assertions(2); const q = new PromiseBuffer(); - const p = new Promise(resolve => setTimeout(resolve, 1)); + const p = new SyncPromise(resolve => setTimeout(resolve, 1)); q.add(p).then(() => { expect(q.length()).toBe(0); }); @@ -37,7 +38,7 @@ describe('PromiseBuffer', () => { test('receive promise result outside and from buffer', async () => { expect.assertions(4); const q = new PromiseBuffer(); - const p = new Promise(resolve => + const p = new SyncPromise(resolve => setTimeout(() => { resolve('test'); }, 1), @@ -57,7 +58,7 @@ describe('PromiseBuffer', () => { expect.assertions(3); const q = new PromiseBuffer(); for (let i = 0; i < 5; i++) { - const p = new Promise(resolve => setTimeout(resolve, 1)); + const p = new SyncPromise(resolve => setTimeout(resolve, 1)); q.add(p); } expect(q.length()).toBe(5); @@ -72,7 +73,7 @@ describe('PromiseBuffer', () => { expect.assertions(2); const q = new PromiseBuffer(); for (let i = 0; i < 5; i++) { - const p = new Promise(resolve => setTimeout(resolve, 100)); + const p = new SyncPromise(resolve => setTimeout(resolve, 100)); q.add(p); } expect(q.length()).toBe(5); @@ -96,7 +97,7 @@ describe('PromiseBuffer', () => { test('rejecting', async () => { expect.assertions(1); const q = new PromiseBuffer(); - const p = new Promise((_, reject) => setTimeout(reject, 1)); + const p = new SyncPromise((_, reject) => setTimeout(reject, 1)); jest.runAllTimers(); return q.add(p).then(null, () => { expect(true).toBe(true);