Replace abuse of promises with the queueMicrotask API in the RPC re…

…quest coalescer (#3374) # Summary The `queueMicrotask` API is well-supported enough now to use for this use case. https://caniuse.com/mdn-api_queuemicrotask
solana-labs · Oct 15, 2024 · 672ca49 · 672ca49
1 parent c45633d
commit 672ca49
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 12 deletions.
diff --git a/packages/rpc/src/__tests__/rpc-request-coalescer-test.ts b/packages/rpc/src/__tests__/rpc-request-coalescer-test.ts
@@ -21,10 +21,10 @@ describe('RPC request coalescer', () => {
             coalescedTransport({ payload: null }).catch(() => {});
             expect(mockTransport).toHaveBeenCalledTimes(1);
         });
-        it('multiple requests in different ticks each produce their own transport request', async () => {
+        it('multiple requests in different ticks each produce their own transport request', () => {
             expect.assertions(1);
             coalescedTransport({ payload: null }).catch(() => {});
-            await jest.runOnlyPendingTimersAsync();
+            jest.runAllTicks();
             coalescedTransport({ payload: null }).catch(() => {});
             expect(mockTransport).toHaveBeenCalledTimes(2);
         });
@@ -46,7 +46,7 @@ describe('RPC request coalescer', () => {
             mockTransport.mockResolvedValueOnce(mockResponseA);
             mockTransport.mockResolvedValueOnce(mockResponseB);
             const responsePromiseA = coalescedTransport({ payload: null });
-            await jest.runOnlyPendingTimersAsync();
+            jest.runAllTicks();
             const responsePromiseB = coalescedTransport({ payload: null });
             await Promise.all([
                 expect(responsePromiseA).resolves.toBe(mockResponseA),
@@ -73,13 +73,13 @@ describe('RPC request coalescer', () => {
             const responsePromiseA = coalescedTransport({ payload: null });
             // eslint-disable-next-line jest/valid-expect
             const expectationA = expect(responsePromiseA).rejects.toBe(mockErrorA);
-            await jest.runOnlyPendingTimersAsync();
+            jest.runAllTicks();
             const responsePromiseB = coalescedTransport({ payload: null });
             // eslint-disable-next-line jest/valid-expect
             const expectationB = expect(responsePromiseB).rejects.toBe(mockErrorB);
             await Promise.all([expectationA, expectationB]);
         });
-        it('does not abort the transport when the number of consumers increases, falls to zero, then increases again in the same runloop', async () => {
+        it('does not abort the transport when the number of consumers increases, falls to zero, then increases again in the same runloop', () => {
             expect.assertions(2);
             const abortControllerA = new AbortController();
             const abortControllerB = new AbortController();
@@ -90,7 +90,7 @@ describe('RPC request coalescer', () => {
             abortControllerB.abort('o no B');
             // New request comes in at the last moment before the end of the runloop.
             coalescedTransport({ payload: null }).catch(() => {});
-            await jest.runOnlyPendingTimersAsync();
+            jest.runAllTicks();
             expect(mockTransport).toHaveBeenCalledTimes(1);
             const transportAbortSignal = mockTransport.mock.lastCall![0].signal!;
             expect(transportAbortSignal.aborted).toBe(false);
@@ -136,13 +136,13 @@ describe('RPC request coalescer', () => {
                 abortControllerA.abort('o no');
                 await expect(responsePromiseA).rejects.toBe('o no');
             });
-            it('aborts the transport at the end of the runloop when all of the requests abort', async () => {
+            it('aborts the transport at the end of the runloop when all of the requests abort', () => {
                 expect.assertions(1);
                 responsePromiseA.catch(() => {});
                 responsePromiseB.catch(() => {});
                 abortControllerA.abort('o no A');
                 abortControllerB.abort('o no B');
-                await jest.runOnlyPendingTimersAsync();
+                jest.runAllTicks();
                 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
                 const transportAbortSignal = mockTransport.mock.lastCall![0].signal!;
                 expect(transportAbortSignal.aborted).toBe(true);

diff --git a/packages/rpc/src/rpc-request-coalescer.ts b/packages/rpc/src/rpc-request-coalescer.ts
@@ -39,8 +39,7 @@ export function getRpcTransportWithRequestCoalescing<TTransport extends RpcTrans
             return await transport(request);
         }
         if (!coalescedRequestsByDeduplicationKey) {
-            // FIXME: Probably replace this with `queueMicrotask()`
-            void Promise.resolve().then(() => {
+            queueMicrotask(() => {
                 coalescedRequestsByDeduplicationKey = undefined;
             });
             coalescedRequestsByDeduplicationKey = {};
@@ -77,8 +76,7 @@ export function getRpcTransportWithRequestCoalescing<TTransport extends RpcTrans
                 const handleAbort = (e: AbortSignalEventMap['abort']) => {
                     signal.removeEventListener('abort', handleAbort);
                     coalescedRequest.numConsumers -= 1;
-                    // FIXME: Probably replace this with `queueMicrotask()`
-                    void Promise.resolve().then(() => {
+                    queueMicrotask(() => {
                         if (coalescedRequest.numConsumers === 0) {
                             const abortController = coalescedRequest.abortController;
                             abortController.abort((EXPLICIT_ABORT_TOKEN ||= createExplicitAbortToken()));