Skip to content

Commit

Permalink
useLazyQuery: fix rules of React violations (#11851)
Browse files Browse the repository at this point in the history
* useLazyQuery: fix rules of React violations

* size-limits

* Clean up Prettier, Size-limit, and Api-Extractor

---------

Co-authored-by: phryneas <phryneas@users.noreply.github.com>
  • Loading branch information
phryneas and phryneas committed May 21, 2024
1 parent 8740f19 commit 45c47be
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/shaggy-mirrors-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Avoid usage of useRef in useInternalState to prevent ref access in render.
5 changes: 5 additions & 0 deletions .changeset/stupid-planes-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix a bug where `useLazyQuery` would not pick up a client change.
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -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
}
26 changes: 15 additions & 11 deletions src/react/hooks/useLazyQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type {
LazyQueryHookOptions,
LazyQueryResultTuple,
NoInfer,
QueryResult,
} from "../types/types.js";
import { useInternalState } from "./useQuery.js";
import { useApolloClient } from "./useApolloClient.js";
Expand Down Expand Up @@ -95,30 +94,35 @@ export function useLazyQuery<
useQueryResult.observable.options.initialFetchPolicy ||
internalState.getDefaultFetchPolicy();

const result: QueryResult<TData, TVariables> = 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<string, any> = {};
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);
};
}

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<LazyQueryResultTuple<TData, TVariables>[0]>(
(executeOptions) => {
Expand Down Expand Up @@ -147,7 +151,7 @@ export function useLazyQuery<

return promise;
},
[]
[eagerMethods, initialFetchPolicy, internalState]
);

return [execute, result];
Expand Down
37 changes: 22 additions & 15 deletions src/react/hooks/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,23 +109,30 @@ export function useInternalState<TData, TVariables extends OperationVariables>(
client: ApolloClient<any>,
query: DocumentNode | TypedDocumentNode<TData, TVariables>
): InternalState<TData, TVariables> {
const stateRef = React.useRef<InternalState<TData, TVariables>>();
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<TData, TVariables>) {
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;
}
Expand Down Expand Up @@ -511,7 +518,7 @@ class InternalState<TData, TVariables extends OperationVariables> {
private onError(error: ApolloError) {}

private observable!: ObservableQuery<TData, TVariables>;
private obsQueryFields!: Omit<
public obsQueryFields!: Omit<
ObservableQueryFields<TData, TVariables>,
"variables"
>;
Expand Down

0 comments on commit 45c47be

Please sign in to comment.