Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: deep merge resolvers #11760

Merged
merged 21 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
5d6a069
feat: deep merge resolvers by default
alessbell Apr 5, 2024
912220a
chore: add changeset
alessbell Apr 5, 2024
12a5f36
chore: update comment
alessbell Apr 8, 2024
0fb94cc
chore: add reset test, remove option
alessbell Apr 10, 2024
696a15d
chore: update changeset
alessbell Apr 10, 2024
49d9557
Merge branch 'release-3.10' into issue-11758-merging-resolvers
alessbell Apr 10, 2024
cd40f9c
Merge branch 'release-3.10' of github.com:apollographql/apollo-client…
alessbell Apr 10, 2024
c10dd8b
chore: update changeset
alessbell Apr 10, 2024
52afa27
Merge branch 'release-3.10' into issue-11758-merging-resolvers
alessbell Apr 10, 2024
ce4f9cd
fix: update function names in tests
alessbell Apr 10, 2024
178eb83
fix: remove unused useTrackRenders
alessbell Apr 12, 2024
b6bf236
Merge branch 'release-3.10' of github.com:apollographql/apollo-client…
alessbell Apr 12, 2024
f51a233
Merge branch 'release-3.10' into issue-11758-merging-resolvers
alessbell Apr 12, 2024
25872f6
chore: fix tests post-merge and merge resolvers in .fork
alessbell Apr 12, 2024
b36407b
Merge branch 'release-3.10' into issue-11758-merging-resolvers
alessbell Apr 12, 2024
b95cad4
chore: comment out flaky test
alessbell Apr 12, 2024
07a568a
Merge branch 'issue-11758-merging-resolvers' of github.com:apollograp…
alessbell Apr 12, 2024
712d338
chore: update comment with link to issue
alessbell Apr 12, 2024
0d49273
commit failing test
alessbell Apr 12, 2024
0130771
chore: fix merge conflicts
alessbell Apr 15, 2024
c6eaea4
chore: remove old comment
alessbell Apr 15, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cold-dancers-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": minor
---

`createTestSchema` now uses graphql-tools `mergeResolvers` to merge resolvers instead of a shallow merge.
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
"@babel/parser": "7.24.1",
"@changesets/changelog-github": "0.5.0",
"@changesets/cli": "2.27.1",
"@graphql-tools/merge": "^9.0.3",
"@graphql-tools/schema": "10.0.3",
"@graphql-tools/utils": "10.0.13",
"@microsoft/api-extractor": "7.42.3",
Expand Down
214 changes: 209 additions & 5 deletions src/testing/core/__tests__/createTestSchema.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ describe("schema proxy", () => {
unmount();
});

it("does not pollute the original schema", async () => {
it("schema.fork does not pollute the original schema", async () => {
alessbell marked this conversation as resolved.
Show resolved Hide resolved
const Profiler = createDefaultProfiler<ViewerQueryData>();

using _fetch = createSchemaFetch(schema);
Expand Down Expand Up @@ -876,10 +876,9 @@ describe("schema proxy", () => {
resolvers: {
Query: {
viewer: () => ({
name: "Virginia",
book: {
colors: ["red", "blue", "green"],
title: "The Book",
title: "A New Book",
},
}),
},
Expand Down Expand Up @@ -997,7 +996,7 @@ describe("schema proxy", () => {
colors: ["red", "blue", "green"],
id: "1",
publishedAt: "2024-01-01",
title: "The Book",
title: "A New Book",
},
},
});
Expand All @@ -1020,12 +1019,217 @@ describe("schema proxy", () => {
colors: ["red", "blue", "green"],
id: "1",
publishedAt: "2024-01-01",
title: "The Book",
title: "A New Book",
},
},
});
}

unmount();
});

describe("schema.reset", () => {
const resetTestSchema = createTestSchema(schemaWithMocks, {
Query: {
viewer: () => ({
book: {
text: "Hello World",
title: "Orlando: A Biography",
},
}),
},
Book: {
__resolveType: (obj) => {
if ("text" in obj) {
return "TextBook";
}
if ("colors" in obj) {
return "ColoringBook";
}
throw new Error("Could not resolve type");
},
},
});
it("setup test where we add resolvers to schema", async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I totally understand why you setup these tests this way, I'm not super fond that these tests require a specific order in that this one needs to run before the other one to pass. Rather than separate it blocks, I'd advocate for combining them into a single bigger test that demonstrates that calling .add, then .reset will successfully reset back to the original resolvers.

I think all I'd add in the component setup for your tests is a <button > that calls refetch on the useSuspenseQuery that you can use to retrigger the network requests.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 0d49273

cc'ing @phryneas again here because the final test is failing with Exceeded timeout waiting for next render. I've brought over the profiler fix from the other PR, and haven't been able to debug this yet...

const Profiler = createDefaultProfiler<ViewerQueryData>();

resetTestSchema.add({
resolvers: {
Query: {
viewer: () => ({
book: {
text: "Hello World",
title: "The Waves",
},
}),
},
},
});

using _fetch = createSchemaFetch(resetTestSchema);

const client = new ApolloClient({
cache: new InMemoryCache(),
uri,
});

const query: TypedDocumentNode<ViewerQueryData> = gql`
query {
viewer {
id
name
age
book {
id
title
publishedAt
... on ColoringBook {
colors
}
}
}
}
`;

const Fallback = () => {
useTrackRenders();
alessbell marked this conversation as resolved.
Show resolved Hide resolved
return <div>Loading...</div>;
};

const App = () => {
return (
<React.Suspense fallback={<Fallback />}>
<Child />
</React.Suspense>
);
};

const Child = () => {
const result = useSuspenseQuery(query);

useTrackRenders();
alessbell marked this conversation as resolved.
Show resolved Hide resolved

Profiler.mergeSnapshot({
result,
} as Partial<{}>);
alessbell marked this conversation as resolved.
Show resolved Hide resolved

return <div>Hello</div>;
};

const { unmount } = renderWithClient(<App />, {
client,
wrapper: Profiler,
});

// initial suspended render
await Profiler.takeRender();

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

expect(snapshot.result?.data).toEqual({
viewer: {
__typename: "User",
age: 42,
id: "1",
name: "String",
book: {
__typename: "TextBook",
id: "1",
publishedAt: "2024-01-01",
// value set in this test with .add
title: "The Waves",
},
},
});
}

unmount();
alessbell marked this conversation as resolved.
Show resolved Hide resolved
});
it("resets the schema to the original", async () => {
const Profiler = createDefaultProfiler<ViewerQueryData>();

resetTestSchema.reset();

using _fetch = createSchemaFetch(resetTestSchema);

const client = new ApolloClient({
cache: new InMemoryCache(),
uri,
});

const query: TypedDocumentNode<ViewerQueryData> = gql`
query {
viewer {
id
name
age
book {
id
title
publishedAt
... on ColoringBook {
colors
}
}
}
}
`;

const Fallback = () => {
useTrackRenders();
return <div>Loading...</div>;
};

const App = () => {
return (
<React.Suspense fallback={<Fallback />}>
<Child />
</React.Suspense>
);
};

const Child = () => {
const result = useSuspenseQuery(query);

useTrackRenders();

Profiler.mergeSnapshot({
result,
} as Partial<{}>);
alessbell marked this conversation as resolved.
Show resolved Hide resolved

return <div>Hello</div>;
};

const { unmount } = renderWithClient(<App />, {
client,
wrapper: Profiler,
});

// initial suspended render
await Profiler.takeRender();

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

expect(snapshot.result?.data).toEqual({
viewer: {
__typename: "User",
age: 42,
id: "1",
name: "String",
book: {
__typename: "TextBook",
id: "1",
publishedAt: "2024-01-01",
// original value
title: "Orlando: A Biography",
},
},
});
}

unmount();
});
});
});
8 changes: 5 additions & 3 deletions src/testing/core/createTestSchema.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { addResolversToSchema } from "@graphql-tools/schema";
import type { GraphQLSchema } from "graphql";

import { addResolversToSchema } from "@graphql-tools/schema";
import { mergeResolvers } from "@graphql-tools/merge";
import type { Resolvers } from "../../core/types.js";

type ProxiedSchema = GraphQLSchema & ProxiedSchemaFns;
Expand Down Expand Up @@ -55,7 +55,9 @@ const createTestSchema = (

const fns: ProxiedSchemaFns = {
add: ({ resolvers: newResolvers }) => {
targetResolvers = { ...targetResolvers, ...newResolvers };
// @ts-ignore TODO(fixme): IResolvers type does not play well with our Resolvers
targetResolvers = mergeResolvers([targetResolvers, newResolvers]);

targetSchema = addResolversToSchema({
schema: targetSchema,
resolvers: targetResolvers,
Expand Down