From 45c47be26d4e020cfcff359a5af19ccfc39b930e Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 21 May 2024 11:05:48 +0200 Subject: [PATCH] useLazyQuery: fix rules of React violations (#11851) * useLazyQuery: fix rules of React violations * size-limits * Clean up Prettier, Size-limit, and Api-Extractor --------- Co-authored-by: phryneas --- .changeset/shaggy-mirrors-judge.md | 5 ++++ .changeset/stupid-planes-nail.md | 5 ++++ .size-limits.json | 2 +- src/react/hooks/useLazyQuery.ts | 26 ++++++++++++--------- src/react/hooks/useQuery.ts | 37 ++++++++++++++++++------------ 5 files changed, 48 insertions(+), 27 deletions(-) create mode 100644 .changeset/shaggy-mirrors-judge.md create mode 100644 .changeset/stupid-planes-nail.md diff --git a/.changeset/shaggy-mirrors-judge.md b/.changeset/shaggy-mirrors-judge.md new file mode 100644 index 0000000000..f5e599284a --- /dev/null +++ b/.changeset/shaggy-mirrors-judge.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Avoid usage of useRef in useInternalState to prevent ref access in render. diff --git a/.changeset/stupid-planes-nail.md b/.changeset/stupid-planes-nail.md new file mode 100644 index 0000000000..200a00ac26 --- /dev/null +++ b/.changeset/stupid-planes-nail.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix a bug where `useLazyQuery` would not pick up a client change. diff --git a/.size-limits.json b/.size-limits.json index 6dce9510b9..d082ee9265 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 39574, + "dist/apollo-client.min.cjs": 39607, "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32821 } diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 909bf8a24d..a8d6eb00a6 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -9,7 +9,6 @@ import type { LazyQueryHookOptions, LazyQueryResultTuple, NoInfer, - QueryResult, } from "../types/types.js"; import { useInternalState } from "./useQuery.js"; import { useApolloClient } from "./useApolloClient.js"; @@ -95,20 +94,17 @@ export function useLazyQuery< useQueryResult.observable.options.initialFetchPolicy || internalState.getDefaultFetchPolicy(); - const result: QueryResult = Object.assign(useQueryResult, { - called: !!execOptionsRef.current, - }); - + const { forceUpdateState, obsQueryFields } = internalState; // We use useMemo here to make sure the eager methods have a stable identity. const eagerMethods = React.useMemo(() => { const eagerMethods: Record = {}; for (const key of EAGER_METHODS) { - const method = result[key]; + const method = obsQueryFields[key]; eagerMethods[key] = function () { if (!execOptionsRef.current) { execOptionsRef.current = Object.create(null); // Only the first time populating execOptionsRef.current matters here. - internalState.forceUpdateState(); + forceUpdateState(); } // @ts-expect-error this is just too generic to type return method.apply(this, arguments); @@ -116,9 +112,17 @@ export function useLazyQuery< } return eagerMethods; - }, []); - - Object.assign(result, eagerMethods); + }, [forceUpdateState, obsQueryFields]); + + const called = !!execOptionsRef.current; + const result = React.useMemo( + () => ({ + ...useQueryResult, + ...eagerMethods, + called, + }), + [useQueryResult, eagerMethods, called] + ); const execute = React.useCallback[0]>( (executeOptions) => { @@ -147,7 +151,7 @@ export function useLazyQuery< return promise; }, - [] + [eagerMethods, initialFetchPolicy, internalState] ); return [execute, result]; diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 225577521b..c4ed41193e 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -109,23 +109,30 @@ export function useInternalState( client: ApolloClient, query: DocumentNode | TypedDocumentNode ): InternalState { - const stateRef = React.useRef>(); - if ( - !stateRef.current || - client !== stateRef.current.client || - query !== stateRef.current.query - ) { - stateRef.current = new InternalState(client, query, stateRef.current); - } - const state = stateRef.current; - // By default, InternalState.prototype.forceUpdate is an empty function, but // we replace it here (before anyone has had a chance to see this state yet) // with a function that unconditionally forces an update, using the latest - // setTick function. Updating this state by calling state.forceUpdate is the - // only way we trigger React component updates (no other useState calls within - // the InternalState class). - state.forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; + // setTick function. Updating this state by calling state.forceUpdate or the + // uSES notification callback are the only way we trigger React component updates. + const forceUpdateState = React.useReducer((tick) => tick + 1, 0)[1]; + + function createInternalState(previous?: InternalState) { + return Object.assign(new InternalState(client, query, previous), { + forceUpdateState, + }); + } + + let [state, updateState] = React.useState(createInternalState); + + if (client !== state.client || query !== state.query) { + // If the client or query have changed, we need to create a new InternalState. + // This will trigger a re-render with the new state, but it will also continue + // to run the current render function to completion. + // Since we sometimes trigger some side-effects in the render function, we + // re-assign `state` to the new state to ensure that those side-effects are + // triggered with the new state. + updateState((state = createInternalState(state))); + } return state; } @@ -511,7 +518,7 @@ class InternalState { private onError(error: ApolloError) {} private observable!: ObservableQuery; - private obsQueryFields!: Omit< + public obsQueryFields!: Omit< ObservableQueryFields, "variables" >;