From 2c7ae833633cf6a711c9477143d569240488fc6d Mon Sep 17 00:00:00 2001 From: MeetinaXD Date: Tue, 26 Nov 2024 17:22:56 +0800 Subject: [PATCH 1/3] fix: resolve issue with delegateMiddleware using outdated context in react --- .changeset/witty-snakes-yawn.md | 5 + .npmrc | 1 - .../src/middlewares/actionDelegation.ts | 72 ++++---- .../react/actionDelegationMiddleware.spec.tsx | 162 ++++++++++++++++++ 4 files changed, 207 insertions(+), 33 deletions(-) create mode 100644 .changeset/witty-snakes-yawn.md create mode 100644 packages/client/test/react/actionDelegationMiddleware.spec.tsx diff --git a/.changeset/witty-snakes-yawn.md b/.changeset/witty-snakes-yawn.md new file mode 100644 index 00000000..600b37d2 --- /dev/null +++ b/.changeset/witty-snakes-yawn.md @@ -0,0 +1,5 @@ +--- +'alova': patch +--- + +fix: resolve issue with delegateMiddleware using outdated context in react diff --git a/.npmrc b/.npmrc index ae643592..e69de29b 100644 --- a/.npmrc +++ b/.npmrc @@ -1 +0,0 @@ -//registry.npmjs.org/:_authToken=${NPM_TOKEN} diff --git a/packages/client/src/middlewares/actionDelegation.ts b/packages/client/src/middlewares/actionDelegation.ts index 3c598552..88edcf10 100644 --- a/packages/client/src/middlewares/actionDelegation.ts +++ b/packages/client/src/middlewares/actionDelegation.ts @@ -1,15 +1,14 @@ import { statesHookHelper } from '@/util/helper'; import { + $self, createAssert, - falseValue, filterItem, forEach, instanceOf, isNumber, isString, objectKeys, - pushItem, - trueValue + pushItem } from '@alova/shared'; import { AlovaGenerics, promiseStatesHook } from 'alova'; import { @@ -19,6 +18,7 @@ import { AlovaGuardNext } from '~/typings/clienthook'; +let currentHookIndex = 0; const actionsMap: Record = {}; const isFrontMiddlewareContext = ( context: AlovaFrontMiddlewareContext | AlovaFetcherMiddlewareContext @@ -37,9 +37,20 @@ const assert = createAssert('subscriber'); export const actionDelegationMiddleware = ( id: string | number | symbol ) => { - const { ref } = statesHookHelper(promiseStatesHook()); + const { ref, onUnmounted } = statesHookHelper(promiseStatesHook()); + + const hookIndex = ref(currentHookIndex + 1); + + if (hookIndex.current > currentHookIndex) { + currentHookIndex += 1; + } + + onUnmounted(() => { + if (actionsMap[id]?.[hookIndex.current]) { + // TODO delete this action + } + }); - const delegated = ref(falseValue); return ( context: (AlovaFrontMiddlewareContext | AlovaFetcherMiddlewareContext) & { delegatingActions?: Actions; @@ -47,34 +58,31 @@ export const actionDelegationMiddleware = ) => { // The middleware will be called repeatedly. If you have already subscribed, you do not need to subscribe again. - if (!delegated.current) { - const { abort, proxyStates, delegatingActions = {} } = context; - const update = (newStates: Record) => { - type ProxyStateKeys = keyof typeof proxyStates; - for (const key in newStates) { - proxyStates[key as ProxyStateKeys] && (proxyStates[key as ProxyStateKeys].v = newStates[key]); + const { abort, proxyStates, delegatingActions = {} } = context; + const update = (newStates: Record) => { + type ProxyStateKeys = keyof typeof proxyStates; + for (const key in newStates) { + proxyStates[key as ProxyStateKeys] && (proxyStates[key as ProxyStateKeys].v = newStates[key]); + } + }; + // Those with the same ID will be saved together in the form of an array + const hooks = (actionsMap[id] = actionsMap[id] || []); + const handler = isFrontMiddlewareContext(context) + ? { + ...delegatingActions, + send: context.send, + abort, + update } - }; - // Those with the same ID will be saved together in the form of an array - const handlersItems = (actionsMap[id] = actionsMap[id] || []); - handlersItems.push( - isFrontMiddlewareContext(context) - ? { - ...delegatingActions, - send: context.send, - abort, - update - } - : { - ...delegatingActions, - fetch: context.fetch, - abort, - update - } - ); + : { + ...delegatingActions, + fetch: context.fetch, + abort, + update + }; + + hooks[hookIndex.current] = handler; - delegated.current = trueValue; - } return next(); }; }; @@ -107,5 +115,5 @@ export const accessAction = ( assert(false, `no handler can be matched by using \`${id.toString()}\``); } - forEach(matched, onMatch); + forEach(filterItem(matched, $self), onMatch); }; diff --git a/packages/client/test/react/actionDelegationMiddleware.spec.tsx b/packages/client/test/react/actionDelegationMiddleware.spec.tsx new file mode 100644 index 00000000..7e3bc06f --- /dev/null +++ b/packages/client/test/react/actionDelegationMiddleware.spec.tsx @@ -0,0 +1,162 @@ +import { useRequest, actionDelegationMiddleware, accessAction, useWatcher } from '@/index'; +import { createAlova } from 'alova'; +import ReactHook from 'alova/react'; +import { send } from 'process'; +import { untilCbCalled } from 'root/testUtils'; +import { mockRequestAdapter } from '~/test/mockData'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { ReactElement, StrictMode, useState } from 'react'; +import { Result, delay } from 'root/testUtils'; +import { request } from 'http'; + +const StrictModeReact = StrictMode as any; + +const alovaInst = createAlova({ + baseURL: 'http://localhost:8080', + statesHook: ReactHook, + requestAdapter: mockRequestAdapter, + cacheLogger: false +}); + +describe('react => subscriber middleware', () => { + test('should reset context after state changes', async () => { + const successFn1 = vi.fn(); + const successFn2 = vi.fn(); + const request1 = vi.fn(); + const request2 = vi.fn(); + + function Page() { + const [name1, setName1] = useState(0); + const [name2, setName2] = useState(0); + const methodInstance = (params: any) => + alovaInst.Get('/info-list', { + params, + cacheFor: 0 + }); + + const watcher1 = useWatcher( + () => { + request1(name1); + return methodInstance({ name: name1 }); + }, + [name1], + { + middleware: actionDelegationMiddleware('delegate') + } + ); + const watcher2 = useWatcher( + () => { + request2(name2); + return methodInstance({ name: name2 }); + }, + [name2], + { + middleware: actionDelegationMiddleware('delegate') + } + ); + + watcher1.onSuccess(successFn1); + watcher2.onSuccess(successFn2); + + return ( +
+ {watcher1.loading ? 'loading' : 'loaded'} + {watcher2.loading ? 'loading' : 'loaded'} + {name1} + {name2} + + +
+ ); + } + + render( + ( + + + + ) as ReactElement + ); + await waitFor(() => { + expect(screen.getByRole('status-1')).toHaveTextContent('loaded'); + expect(screen.getByRole('status-2')).toHaveTextContent('loaded'); + }); + + // name: 1 0. + fireEvent.click(screen.getByRole('btnSend1')); + + await waitFor(() => { + expect(screen.getByRole('status-1')).toHaveTextContent('loaded'); + expect(screen.getByRole('name-1')).toHaveTextContent('1'); + expect(screen.getByRole('name-2')).toHaveTextContent('0'); + expect(successFn1).toHaveBeenCalledTimes(1); + expect(successFn2).toHaveBeenCalledTimes(0); + expect(request1).toHaveBeenCalledWith(1); + }); + + // name: 1 1 + fireEvent.click(screen.getByRole('btnSend2')); + + await waitFor(() => { + expect(screen.getByRole('status-2')).toHaveTextContent('loaded'); + expect(screen.getByRole('name-1')).toHaveTextContent('1'); + expect(screen.getByRole('name-2')).toHaveTextContent('1'); + expect(successFn1).toHaveBeenCalledTimes(1); + expect(successFn2).toHaveBeenCalledTimes(1); + expect(request2).toHaveBeenCalledWith(1); + }); + + // name: 1 1 + // invoke hooks with middleware id `delegate`, should call watcher1.send and watcher2.send + accessAction('delegate', async ({ send }) => { + send({ fetch: 'delegate' }); + }); + + await waitFor(() => { + expect(screen.getByRole('status-1')).toHaveTextContent('loaded'); + expect(screen.getByRole('status-2')).toHaveTextContent('loaded'); + expect(screen.getByRole('name-1')).toHaveTextContent('1'); + expect(screen.getByRole('name-2')).toHaveTextContent('1'); + expect(successFn1).toHaveBeenCalledTimes(2); + expect(successFn2).toHaveBeenCalledTimes(2); + expect(request1).toHaveBeenCalledWith(1); + expect(request2).toHaveBeenCalledWith(1); + }); + + // name: 1 2 + fireEvent.click(screen.getByRole('btnSend2')); + + await waitFor(() => { + expect(screen.getByRole('status-2')).toHaveTextContent('loaded'); + expect(screen.getByRole('name-1')).toHaveTextContent('1'); + expect(screen.getByRole('name-2')).toHaveTextContent('2'); + expect(successFn1).toHaveBeenCalledTimes(2); + expect(successFn2).toHaveBeenCalledTimes(3); + }); + + // name: 1 2 + // invoke hooks with middleware id `delegate` + accessAction('delegate', async ({ send }) => { + send({ fetch: 'delegate' }); + }); + + await waitFor(() => { + expect(screen.getByRole('status-1')).toHaveTextContent('loaded'); + expect(screen.getByRole('status-2')).toHaveTextContent('loaded'); + expect(screen.getByRole('name-1')).toHaveTextContent('1'); + expect(screen.getByRole('name-2')).toHaveTextContent('2'); + expect(successFn1).toHaveBeenCalledTimes(3); + expect(successFn2).toHaveBeenCalledTimes(4); + expect(request1).toHaveBeenCalledWith(1); + expect(request2).toHaveBeenCalledWith(2); + }); + }); +}); From 595ef01bead6dc6770a13c416060442764136105 Mon Sep 17 00:00:00 2001 From: MeetinaXD Date: Tue, 26 Nov 2024 17:30:02 +0800 Subject: [PATCH 2/3] fix: lint fix --- .../test/react/actionDelegationMiddleware.spec.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/client/test/react/actionDelegationMiddleware.spec.tsx b/packages/client/test/react/actionDelegationMiddleware.spec.tsx index 7e3bc06f..276d1b71 100644 --- a/packages/client/test/react/actionDelegationMiddleware.spec.tsx +++ b/packages/client/test/react/actionDelegationMiddleware.spec.tsx @@ -1,13 +1,9 @@ -import { useRequest, actionDelegationMiddleware, accessAction, useWatcher } from '@/index'; +import { accessAction, actionDelegationMiddleware, useWatcher } from '@/index'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { createAlova } from 'alova'; import ReactHook from 'alova/react'; -import { send } from 'process'; -import { untilCbCalled } from 'root/testUtils'; -import { mockRequestAdapter } from '~/test/mockData'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { ReactElement, StrictMode, useState } from 'react'; -import { Result, delay } from 'root/testUtils'; -import { request } from 'http'; +import { mockRequestAdapter } from '~/test/mockData'; const StrictModeReact = StrictMode as any; From 616961b6dddcf70613fde3a63f669655c58482c2 Mon Sep 17 00:00:00 2001 From: MeetinaXD Date: Tue, 26 Nov 2024 20:55:56 +0800 Subject: [PATCH 3/3] perf: remove handler on unmount --- packages/client/src/middlewares/actionDelegation.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/client/src/middlewares/actionDelegation.ts b/packages/client/src/middlewares/actionDelegation.ts index 88edcf10..5988b0e5 100644 --- a/packages/client/src/middlewares/actionDelegation.ts +++ b/packages/client/src/middlewares/actionDelegation.ts @@ -8,6 +8,7 @@ import { isNumber, isString, objectKeys, + objectValues, pushItem } from '@alova/shared'; import { AlovaGenerics, promiseStatesHook } from 'alova'; @@ -19,7 +20,8 @@ import { } from '~/typings/clienthook'; let currentHookIndex = 0; -const actionsMap: Record = {}; +// (id, (hookIndex, Actions)) +const actionsMap: Record> = {}; const isFrontMiddlewareContext = ( context: AlovaFrontMiddlewareContext | AlovaFetcherMiddlewareContext ): context is AlovaFrontMiddlewareContext => !!(context as AlovaFrontMiddlewareContext).send; @@ -47,7 +49,8 @@ export const actionDelegationMiddleware = { if (actionsMap[id]?.[hookIndex.current]) { - // TODO delete this action + // delete action on unmount + delete actionsMap[id][hookIndex.current]; } }); @@ -100,12 +103,12 @@ export const accessAction = ( ) => { const matched = [] as Actions[]; if (typeof id === 'symbol' || isString(id) || isNumber(id)) { - actionsMap[id] && pushItem(matched, ...actionsMap[id]); + actionsMap[id] && pushItem(matched, ...objectValues(actionsMap[id])); } else if (instanceOf(id, RegExp)) { forEach( filterItem(objectKeys(actionsMap), idItem => id.test(idItem)), idItem => { - pushItem(matched, ...actionsMap[idItem]); + pushItem(matched, ...objectValues(actionsMap[idItem])); } ); }