Skip to content

Commit

Permalink
Merge branch 'main' into renovate/all-patch
Browse files Browse the repository at this point in the history
  • Loading branch information
phryneas authored Mar 6, 2024
2 parents 3581112 + e9fd314 commit 9cf3adf
Show file tree
Hide file tree
Showing 18 changed files with 781 additions and 113 deletions.
397 changes: 391 additions & 6 deletions .api-reports/api-report-react_internal.md

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions .changeset/curvy-maps-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Allow Apollo Client instance to intercept hook functionality
5 changes: 5 additions & 0 deletions .changeset/rich-hotels-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

Fix issue where calling `fetchMore` from a suspense-enabled hook inside `startTransition` caused an unnecessary rerender.
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": 39075,
"dist/apollo-client.min.cjs": 39211,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32584
}
4 changes: 3 additions & 1 deletion config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import cleanup from "rollup-plugin-cleanup";
const entryPoints = require("./entryPoints");
const distDir = "./dist";

const removeComments = cleanup({});
const removeComments = cleanup({
comments: ["some", /#__PURE__/, /#__NO_SIDE_EFFECTS__/],
});

function isExternal(id, parentId, entryPointsAreExternal = true) {
let posixId = toPosixPath(id);
Expand Down
82 changes: 60 additions & 22 deletions docs/source/data/suspense.mdx

Large diffs are not rendered by default.

71 changes: 0 additions & 71 deletions src/react/hooks/__tests__/useBackgroundQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4927,25 +4927,6 @@ describe("fetchMore", () => {
expect(renderedComponents).toStrictEqual([App, SuspenseFallback]);
}

// TODO: Determine why we have this extra render here.
// Possibly related: https://github.com/apollographql/apollo-client/issues/11315
{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: {
letters: [
{ __typename: "Letter", position: 1, letter: "A" },
{ __typename: "Letter", position: 2, letter: "B" },
{ __typename: "Letter", position: 3, letter: "C" },
{ __typename: "Letter", position: 4, letter: "D" },
],
},
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

{
const { snapshot } = await Profiler.takeRender();

Expand Down Expand Up @@ -5034,25 +5015,6 @@ describe("fetchMore", () => {
expect(renderedComponents).toStrictEqual([App, SuspenseFallback]);
}

// TODO: Determine why we have this extra render here.
// Possibly related: https://github.com/apollographql/apollo-client/issues/11315
{
const { snapshot } = await Profiler.takeRender();

expect(snapshot.result).toEqual({
data: {
letters: [
{ __typename: "Letter", position: 1, letter: "A" },
{ __typename: "Letter", position: 2, letter: "B" },
{ __typename: "Letter", position: 3, letter: "C" },
{ __typename: "Letter", position: 4, letter: "D" },
],
},
error: undefined,
networkStatus: NetworkStatus.ready,
});
}

{
const { snapshot } = await Profiler.takeRender();

Expand Down Expand Up @@ -5245,39 +5207,6 @@ describe("fetchMore", () => {
});
}

// TODO: Determine why we have this extra render here. This should mimic
// the update in the next render where we see <App /> included in the
// rerendered components.
// Possibly related: https://github.com/apollographql/apollo-client/issues/11315
{
const { snapshot, renderedComponents } = await Profiler.takeRender();

expect(renderedComponents).toStrictEqual([ReadQueryHook]);
expect(snapshot).toEqual({
isPending: false,
result: {
data: {
todos: [
{
__typename: "Todo",
id: "1",
name: "Clean room",
completed: false,
},
{
__typename: "Todo",
id: "2",
name: "Take out trash",
completed: true,
},
],
},
error: undefined,
networkStatus: NetworkStatus.ready,
},
});
}

{
// Eventually we should see the updated todos content once its done
// suspending.
Expand Down
135 changes: 133 additions & 2 deletions src/react/hooks/__tests__/useSuspenseQuery.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { Fragment, StrictMode, Suspense } from "react";
import React, { Fragment, StrictMode, Suspense, useTransition } from "react";
import {
act,
screen,
Expand Down Expand Up @@ -51,7 +51,15 @@ import {
RefetchWritePolicy,
WatchQueryFetchPolicy,
} from "../../../core/watchQueryOptions";
import { profile, spyOnConsole } from "../../../testing/internal";
import {
PaginatedCaseData,
PaginatedCaseVariables,
createProfiler,
profile,
setupPaginatedCase,
spyOnConsole,
useTrackRenders,
} from "../../../testing/internal";

type RenderSuspenseHookOptions<Props, TSerializedCache = {}> = Omit<
RenderHookOptions<Props>,
Expand Down Expand Up @@ -9978,6 +9986,129 @@ describe("useSuspenseQuery", () => {
});
});

// https://github.com/apollographql/apollo-client/issues/11315
it("fetchMore does not cause extra render", async () => {
const { query, link } = setupPaginatedCase();

const user = userEvent.setup();
const client = new ApolloClient({
cache: new InMemoryCache({
typePolicies: {
Query: {
fields: {
letters: offsetLimitPagination(),
},
},
},
}),
link,
});

const Profiler = createProfiler({
initialSnapshot: {
result: null as UseSuspenseQueryResult<
PaginatedCaseData,
PaginatedCaseVariables
> | null,
},
});

function SuspenseFallback() {
useTrackRenders();

return <div>Loading...</div>;
}

function App() {
useTrackRenders();
const [isPending, startTransition] = useTransition();
const result = useSuspenseQuery(query, {
variables: { offset: 0, limit: 2 },
});
const { data, fetchMore } = result;

Profiler.mergeSnapshot({ result });

return (
<button
disabled={isPending}
onClick={() =>
startTransition(() => {
fetchMore({
variables: {
offset: data.letters.length,
limit: data.letters.length + 1,
},
});
})
}
>
Fetch next
</button>
);
}

render(<App />, {
wrapper: ({ children }) => (
<ApolloProvider client={client}>
<Profiler>
<Suspense fallback={<SuspenseFallback />}>{children}</Suspense>
</Profiler>
</ApolloProvider>
),
});

{
const { renderedComponents } = await Profiler.takeRender();

expect(renderedComponents).toStrictEqual([SuspenseFallback]);
}

{
const { snapshot, renderedComponents } = await Profiler.takeRender();

expect(renderedComponents).toStrictEqual([App]);
expect(snapshot.result?.data).toEqual({
letters: [
{ __typename: "Letter", letter: "A", position: 1 },
{ __typename: "Letter", letter: "B", position: 2 },
],
});
}

await act(() => user.click(screen.getByText("Fetch next")));

{
const { snapshot, renderedComponents } = await Profiler.takeRender();

expect(renderedComponents).toStrictEqual([App]);
expect(screen.getByText("Fetch next")).toBeDisabled();
expect(snapshot.result?.data).toEqual({
letters: [
{ __typename: "Letter", letter: "A", position: 1 },
{ __typename: "Letter", letter: "B", position: 2 },
],
});
}

{
const { snapshot, renderedComponents } = await Profiler.takeRender();

expect(renderedComponents).toStrictEqual([App]);
expect(snapshot.result?.data).toEqual({
letters: [
{ __typename: "Letter", letter: "A", position: 1 },
{ __typename: "Letter", letter: "B", position: 2 },
{ __typename: "Letter", letter: "C", position: 3 },
{ __typename: "Letter", letter: "D", position: 4 },
{ __typename: "Letter", letter: "E", position: 5 },
],
});
}

await expect(Profiler).not.toRerender();
});

describe.skip("type tests", () => {
it("returns unknown when TData cannot be inferred", () => {
const query = gql`
Expand Down
1 change: 1 addition & 0 deletions src/react/hooks/internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export { useIsomorphicLayoutEffect } from "./useIsomorphicLayoutEffect.js";
export { useRenderGuard } from "./useRenderGuard.js";
export { useLazyRef } from "./useLazyRef.js";
export { __use } from "./__use.js";
export { wrapHook } from "./wrapHook.js";
88 changes: 88 additions & 0 deletions src/react/hooks/internal/wrapHook.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import type {
useQuery,
useSuspenseQuery,
useBackgroundQuery,
useReadQuery,
useFragment,
} from "../index.js";
import type { QueryManager } from "../../../core/QueryManager.js";
import type { ApolloClient } from "../../../core/ApolloClient.js";
import type { ObservableQuery } from "../../../core/ObservableQuery.js";

const wrapperSymbol = Symbol.for("apollo.hook.wrappers");

interface WrappableHooks {
useQuery: typeof useQuery;
useSuspenseQuery: typeof useSuspenseQuery;
useBackgroundQuery: typeof useBackgroundQuery;
useReadQuery: typeof useReadQuery;
useFragment: typeof useFragment;
}

/**
* @internal
* Can be used to correctly type the [Symbol.for("apollo.hook.wrappers")] property of
* `QueryManager`, to override/wrap hook functionality.
*/
export type HookWrappers = {
[K in keyof WrappableHooks]?: (
originalHook: WrappableHooks[K]
) => WrappableHooks[K];
};

interface QueryManagerWithWrappers<T> extends QueryManager<T> {
[wrapperSymbol]?: HookWrappers;
}

/**
* @internal
*
* Makes an Apollo Client hook "wrappable".
* That means that the Apollo Client instance can expose a "wrapper" that will be
* used to wrap the original hook implementation with additional logic.
* @example
* ```tsx
* // this is already done in `@apollo/client` for all wrappable hooks (see `WrappableHooks`)
* // following this pattern
* function useQuery() {
* return wrapHook('useQuery', _useQuery, options.client)(query, options);
* }
* function _useQuery(query, options) {
* // original implementation
* }
*
* // this is what a library like `@apollo/client-react-streaming` would do
* class ApolloClientWithStreaming extends ApolloClient {
* constructor(options) {
* super(options);
* this.queryManager[Symbol.for("apollo.hook.wrappers")] = {
* useQuery: (original) => (query, options) => {
* console.log("useQuery was called with options", options);
* return original(query, options);
* }
* }
* }
* }
*
* // this will now log the options and then call the original `useQuery`
* const client = new ApolloClientWithStreaming({ ... });
* useQuery(query, { client });
* ```
*/
export function wrapHook<Hook extends (...args: any[]) => any>(
hookName: keyof WrappableHooks,
useHook: Hook,
clientOrObsQuery: ObservableQuery<any> | ApolloClient<any>
): Hook {
const queryManager = (
clientOrObsQuery as unknown as {
// both `ApolloClient` and `ObservableQuery` have a `queryManager` property
// but they're both `private`, so we have to cast around for a bit here.
queryManager: QueryManagerWithWrappers<any>;
}
)["queryManager"];
const wrappers = queryManager && queryManager[wrapperSymbol];
const wrapper: undefined | ((wrap: Hook) => Hook) =
wrappers && (wrappers[hookName] as any);
return wrapper ? wrapper(useHook) : useHook;
}
Loading

0 comments on commit 9cf3adf

Please sign in to comment.