From c1170e095589f67fe24961781d917862667681f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ti=E1=BA=BFn=20Nguy=E1=BB=85n=20Kh=E1=BA=AFc?= Date: Thu, 6 Mar 2025 19:36:33 +1300 Subject: [PATCH] perf(react): reduce unnecessary promise creation --- .changeset/whole-hounds-unite.md | 5 ++ packages/react/package.json | 1 + .../react/src/hooks/use-chain-spec-data.ts | 12 ++--- packages/react/src/hooks/use-mutation.ts | 2 +- packages/react/src/hooks/use-query.ts | 43 +++++++++-------- packages/react/src/hooks/use-typed-api.ts | 9 ++-- .../atom-family-with-error-catcher.test.ts | 14 +++++- .../react/src/utils/maybe-promise-all.test.ts | 47 +++++++++++++++++++ packages/react/src/utils/maybe-promise-all.ts | 7 +++ yarn.lock | 18 +++++-- 10 files changed, 121 insertions(+), 37 deletions(-) create mode 100644 .changeset/whole-hounds-unite.md create mode 100644 packages/react/src/utils/maybe-promise-all.test.ts create mode 100644 packages/react/src/utils/maybe-promise-all.ts diff --git a/.changeset/whole-hounds-unite.md b/.changeset/whole-hounds-unite.md new file mode 100644 index 00000000..f132731d --- /dev/null +++ b/.changeset/whole-hounds-unite.md @@ -0,0 +1,5 @@ +--- +"@reactive-dot/react": patch +--- + +Reduced unnecessary promise creation. diff --git a/packages/react/package.json b/packages/react/package.json index 2a40398f..ffa13a30 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -34,6 +34,7 @@ "dependencies": { "@reactive-dot/core": "workspace:^", "jotai": "^2.12.1", + "jotai-derive": "^0.1.2", "jotai-effect": "^2.0.1" }, "devDependencies": { diff --git a/packages/react/src/hooks/use-chain-spec-data.ts b/packages/react/src/hooks/use-chain-spec-data.ts index 52e75dd7..33e85b11 100644 --- a/packages/react/src/hooks/use-chain-spec-data.ts +++ b/packages/react/src/hooks/use-chain-spec-data.ts @@ -4,8 +4,8 @@ import { useAtomValue } from "./use-atom-value.js"; import { internal_useChainId } from "./use-chain-id.js"; import { clientAtom } from "./use-client.js"; import { useConfig } from "./use-config.js"; -import type { Config, ChainId } from "@reactive-dot/core"; -import { atom } from "jotai"; +import type { ChainId, Config } from "@reactive-dot/core"; +import { derive } from "jotai-derive"; /** * Hook for fetching the [JSON-RPC spec](https://paritytech.github.io/json-rpc-interface-spec/api/chainSpec.html). @@ -25,10 +25,8 @@ export function useChainSpecData(options?: ChainHookOptions) { export const chainSpecDataAtom = atomFamilyWithErrorCatcher( (withErrorCatcher, config: Config, chainId: ChainId) => withErrorCatcher( - atom(async (get) => { - const client = await get(clientAtom(config, chainId)); - - return client.getChainSpecData(); - }), + derive([clientAtom(config, chainId)], (client) => + client.getChainSpecData(), + ), ), ); diff --git a/packages/react/src/hooks/use-mutation.ts b/packages/react/src/hooks/use-mutation.ts index 3e39a0b1..0f44c14d 100644 --- a/packages/react/src/hooks/use-mutation.ts +++ b/packages/react/src/hooks/use-mutation.ts @@ -79,7 +79,7 @@ export function useMutation< const id = globalThis.crypto.randomUUID(); - return from(get(typedApiAtom(config, chainId))).pipe( + return from(Promise.resolve(get(typedApiAtom(config, chainId)))).pipe( switchMap((typedApi) => { const transaction = action(typedApi.tx); diff --git a/packages/react/src/hooks/use-query.ts b/packages/react/src/hooks/use-query.ts index 216089c2..f9b3e34c 100644 --- a/packages/react/src/hooks/use-query.ts +++ b/packages/react/src/hooks/use-query.ts @@ -2,6 +2,7 @@ import { findAllIndexes } from "../utils/find-all-indexes.js"; import { interlace } from "../utils/interlace.js"; import { atomFamilyWithErrorCatcher } from "../utils/jotai/atom-family-with-error-catcher.js"; import { atomWithObservableAndPromise } from "../utils/jotai/atom-with-observable-and-promise.js"; +import { maybePromiseAll } from "../utils/maybe-promise-all.js"; import { objectId } from "../utils/object-id.js"; import type { ChainHookOptions, @@ -28,6 +29,7 @@ import { } from "@reactive-dot/core/internal.js"; import { preflight, query } from "@reactive-dot/core/internal/actions.js"; import { atom } from "jotai"; +import { soon } from "jotai-derive"; import { atomWithRefresh } from "jotai/utils"; import { useMemo } from "react"; import { from, type Observable } from "rxjs"; @@ -185,11 +187,11 @@ const instructionPayloadAtom = atomFamilyWithErrorCatcher( switch (preflight(instruction)) { case "promise": { const atom = withErrorCatcher( - atomWithRefresh(async (get, { signal }) => { - const api = await get(typedApiAtom(config, chainId)); - - return query(api, instruction, { signal }); - }), + atomWithRefresh((get, { signal }) => + soon(get(typedApiAtom(config, chainId)), (api) => + query(api, instruction, { signal }), + ), + ), ); return { @@ -200,7 +202,7 @@ const instructionPayloadAtom = atomFamilyWithErrorCatcher( case "observable": return atomWithObservableAndPromise( (get) => - from(get(typedApiAtom(config, chainId))).pipe( + from(Promise.resolve(get(typedApiAtom(config, chainId)))).pipe( switchMap( (api) => query(api, instruction) as Observable, ), @@ -257,23 +259,26 @@ export const queryPayloadAtom = atomFamilyWithErrorCatcher( const createAtom = (asObservable: boolean) => withErrorCatcher( atom((get) => { - return Promise.all( + return maybePromiseAll( atoms.map((atomOrAtoms) => !Array.isArray(atomOrAtoms) ? atomOrAtoms - : Promise.all( - atomOrAtoms.map((atomOrAtoms) => { - if (Array.isArray(atomOrAtoms)) { - return Promise.all( - atomOrAtoms.map((atom) => - get(unwrap(atom, asObservable)), - ), - ); - } + : soon( + maybePromiseAll( + atomOrAtoms.map((atomOrAtoms) => { + if (Array.isArray(atomOrAtoms)) { + return maybePromiseAll( + atomOrAtoms.map((atom) => + get(unwrap(atom, asObservable)), + ), + ); + } - return get(unwrap(atomOrAtoms, asObservable)); - }), - ).then(flatHead), + return get(unwrap(atomOrAtoms, asObservable)); + }), + ), + flatHead, + ), ), ); }), diff --git a/packages/react/src/hooks/use-typed-api.ts b/packages/react/src/hooks/use-typed-api.ts index e975ed7a..ef82e3c5 100644 --- a/packages/react/src/hooks/use-typed-api.ts +++ b/packages/react/src/hooks/use-typed-api.ts @@ -11,6 +11,7 @@ import { } from "@reactive-dot/core"; import type { ChainDescriptorOf } from "@reactive-dot/core/internal.js"; import { atom } from "jotai"; +import { soon } from "jotai-derive"; import type { TypedApi } from "polkadot-api"; /** @@ -33,16 +34,16 @@ export function useTypedApi( export const typedApiAtom = atomFamilyWithErrorCatcher( (withErrorCatcher, config: Config, chainId: ChainId) => withErrorCatcher( - atom(async (get) => { + atom((get) => { const chainConfig = config.chains[chainId]; if (chainConfig === undefined) { throw new ReactiveDotError(`No config provided for chain ${chainId}`); } - const client = await get(clientAtom(config, chainId)); - - return client.getTypedApi(chainConfig.descriptor); + return soon(get(clientAtom(config, chainId)), (client) => + client.getTypedApi(chainConfig.descriptor), + ); }), ), ); diff --git a/packages/react/src/utils/jotai/atom-family-with-error-catcher.test.ts b/packages/react/src/utils/jotai/atom-family-with-error-catcher.test.ts index 45abceeb..0c05b51b 100644 --- a/packages/react/src/utils/jotai/atom-family-with-error-catcher.test.ts +++ b/packages/react/src/utils/jotai/atom-family-with-error-catcher.test.ts @@ -1,8 +1,11 @@ -import { atomFamilyWithErrorCatcher } from "./atom-family-with-error-catcher.js"; +import { + atomFamilyErrorsAtom, + atomFamilyWithErrorCatcher, +} from "./atom-family-with-error-catcher.js"; import { atom, createStore } from "jotai"; import { atomWithObservable } from "jotai/utils"; import { of, throwError } from "rxjs"; -import { beforeEach, expect, it } from "vitest"; +import { afterEach, beforeEach, expect, it } from "vitest"; let store: ReturnType; @@ -10,6 +13,10 @@ beforeEach(() => { store = createStore(); }); +afterEach(() => { + store.get(atomFamilyErrorsAtom).clear(); +}); + it("should create an atom family", () => { const myAtomFamily = atomFamilyWithErrorCatcher( (withErrorCatcher, param: string) => { @@ -53,6 +60,7 @@ it("should catch errors in synchronous reads", () => { expect(store.get(myAtomFamily("World"))).toBe("Hello World"); expect(() => store.get(myAtomFamily("Error"))).toThrow("Intentional Error"); + expect(store.get(atomFamilyErrorsAtom).size).toBe(1); }); it("should catch errors in Promise reads", async () => { @@ -74,6 +82,7 @@ it("should catch errors in Promise reads", async () => { await expect(() => store.get(myAtomFamily("Error"))).rejects.toThrow( "Intentional Promise Error", ); + expect(store.get(atomFamilyErrorsAtom).size).toBe(1); }); it("should catch errors in Observable reads", async () => { @@ -93,4 +102,5 @@ it("should catch errors in Observable reads", async () => { expect(() => store.get(myAtomFamily("Error"))).toThrow( "Intentional Observable Error", ); + expect(store.get(atomFamilyErrorsAtom).size).toBe(1); }); diff --git a/packages/react/src/utils/maybe-promise-all.test.ts b/packages/react/src/utils/maybe-promise-all.test.ts new file mode 100644 index 00000000..50594519 --- /dev/null +++ b/packages/react/src/utils/maybe-promise-all.test.ts @@ -0,0 +1,47 @@ +import { maybePromiseAll } from "./maybe-promise-all.js"; +import { expect, it } from "vitest"; + +it("returns array as is when no promises are present", () => { + const input = [1, 2, 3, "string", {}, []]; + const result = maybePromiseAll(input); + + expect(result).toBe(input); +}); + +it("returns Promise.all result when all items are promises", async () => { + const input = [Promise.resolve(1), Promise.resolve(2), Promise.resolve(3)]; + const result = maybePromiseAll(input); + + expect(result).toBeInstanceOf(Promise); + await expect(result).resolves.toEqual([1, 2, 3]); +}); + +it("returns Promise.all result when some items are promises", async () => { + const input = [1, Promise.resolve(2), 3]; + const result = maybePromiseAll(input); + + expect(result).toBeInstanceOf(Promise); + await expect(result).resolves.toEqual([1, 2, 3]); +}); + +it("handles an empty array", () => { + const input: unknown[] = []; + const result = maybePromiseAll(input); + + expect(result).toBe(input); +}); + +it("handles array with undefined/null values", () => { + const input = [undefined, null]; + const result = maybePromiseAll(input); + + expect(result).toBe(input); +}); + +it("propagates rejection when a promise rejects", async () => { + const error = new Error("Test error"); + const input = [Promise.resolve(1), Promise.reject(error), Promise.resolve(3)]; + const result = maybePromiseAll(input); + + await expect(result).rejects.toBe(error); +}); diff --git a/packages/react/src/utils/maybe-promise-all.ts b/packages/react/src/utils/maybe-promise-all.ts new file mode 100644 index 00000000..c61068a8 --- /dev/null +++ b/packages/react/src/utils/maybe-promise-all.ts @@ -0,0 +1,7 @@ +export function maybePromiseAll>( + maybePromises: T[], +) { + return maybePromises.some((maybePromise) => maybePromise instanceof Promise) + ? Promise.all(maybePromises) + : maybePromises; +} diff --git a/yarn.lock b/yarn.lock index 9d613167..18297e94 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5018,6 +5018,7 @@ __metadata: "@types/react": "npm:^19.0.10" eslint: "npm:^9.21.0" jotai: "npm:^2.12.1" + jotai-derive: "npm:^0.1.2" jotai-effect: "npm:^2.0.1" jsdom: "npm:^26.0.0" react: "npm:^19.0.0" @@ -13112,6 +13113,15 @@ __metadata: languageName: node linkType: hard +"jotai-derive@npm:^0.1.2": + version: 0.1.2 + resolution: "jotai-derive@npm:0.1.2" + peerDependencies: + jotai: ">=2.0.0" + checksum: 10c0/72ad1769dcdcd08edae2c6e829bc51313653edbb3693cb271e43dde8779239967a80d5d0af56cca786097c395213b629aff08a10aed31d13bdfcc666aadd1ace + languageName: node + linkType: hard + "jotai-devtools@npm:^0.11.0": version: 0.11.0 resolution: "jotai-devtools@npm:0.11.0" @@ -19668,21 +19678,21 @@ __metadata: "typescript@patch:typescript@npm%3A^5.7.3#optional!builtin": version: 5.7.3 - resolution: "typescript@patch:typescript@npm%3A5.7.3#optional!builtin::version=5.7.3&hash=5786d5" + resolution: "typescript@patch:typescript@npm%3A5.7.3#optional!builtin::version=5.7.3&hash=74658d" bin: tsc: bin/tsc tsserver: bin/tsserver - checksum: 10c0/6fd7e0ed3bf23a81246878c613423730c40e8bdbfec4c6e4d7bf1b847cbb39076e56ad5f50aa9d7ebd89877999abaee216002d3f2818885e41c907caaa192cc4 + checksum: 10c0/3b56d6afa03d9f6172d0b9cdb10e6b1efc9abc1608efd7a3d2f38773d5d8cfb9bbc68dfb72f0a7de5e8db04fc847f4e4baeddcd5ad9c9feda072234f0d788896 languageName: node linkType: hard "typescript@patch:typescript@npm%3A^5.8.2#optional!builtin": version: 5.8.2 - resolution: "typescript@patch:typescript@npm%3A5.8.2#optional!builtin::version=5.8.2&hash=5786d5" + resolution: "typescript@patch:typescript@npm%3A5.8.2#optional!builtin::version=5.8.2&hash=74658d" bin: tsc: bin/tsc tsserver: bin/tsserver - checksum: 10c0/5448a08e595cc558ab321e49d4cac64fb43d1fa106584f6ff9a8d8e592111b373a995a1d5c7f3046211c8a37201eb6d0f1566f15cdb7a62a5e3be01d087848e2 + checksum: 10c0/8a6cd29dfb59bd5a978407b93ae0edb530ee9376a5b95a42ad057a6f80ffb0c410489ccd6fe48d1d0dfad6e8adf5d62d3874bbd251f488ae30e11a1ce6dabd28 languageName: node linkType: hard