feat(gql): support DocumentNode and TypedDocumentNode #774

longility · 2021-06-08T21:42:58Z

Allows consistent usage of TypedDocumentNode between graphQL use and mocking.

References to TypedDocumentNode

closes #773

codesandbox-ci · 2021-06-08T21:51:47Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f473514:

Sandbox	Source
MSW React	Configuration

longility · 2021-06-09T11:11:35Z

src/graphql.test.ts

+
+    // MANUAL VERIFICATION: intellisense on `ctx.data` should be of type `DataContext<GetUserDetailQuery>`.
+    // Not sure how to test this.
+    ctx.data


Manual verification:

longility · 2021-06-09T11:11:59Z

src/graphql.test.ts

+
+    // MANUAL VERIFICATION: intellisense on `ctx.data` should be of type `DataContext<LoginMutation>`.
+    // Not sure how to test this.
+    ctx.data


Manual verification:

We should move this into the test/typings/graphql.test-d.ts, eventually.

longility · 2021-06-09T13:59:39Z

@kettanaito If there are preferred ways or better ways of doing something, let's discuss. I was not completely comfortable with the changes. I used judgment or took a stab at it.

kettanaito · 2021-06-12T11:19:05Z

Hey, @longility. Your changes look great! Please, give me some time to go through them and provide feedback.

kettanaito · 2021-06-12T11:22:26Z

test/typings/graphql.test-d.ts

+    }
+  }
+`)
+graphql.query(getUser, (req, res, ctx) => res(ctx.data({})))


@longility, we use this TypeScript module to test types. It's going to be compiled during the test run and if there are any type of violations, the compilation will fail. We can expect failures to test negative scenarios via @ts-expect-error. This way we make sure there are no type regressions throughout the releases.

src/handlers/GraphQLHandler.test.ts

kettanaito · 2021-06-12T11:25:44Z

package.json

@@ -64,6 +64,7 @@
  ],
  "sideEffects": false,
  "dependencies": {
+    "@graphql-typed-document-node/core": "^3.1.0",


@longility, do you think there's a way to support typed document nodes without introducing this as a dependency?

Not all the users will use GraphQL, and even smaller subset of those who do will use TypedDocumentNode. Adding this as a fixed dependency is quite a price everybody will pay.

From the type definitions, TypedDocumentNode and DocumentNode seem compatible. The only issue I see is that one cannot extract the query fields from DocumentNode straight away, while TypedDocumentNode has generics to infer them in a handler.

Do you think we could swap TypedDocumentNode with the plain DocumentNode (users already install graphql)? We can write a custom extract type to get the query fields from the plain document node to achieve type safety, I think.

We can still use this package as a dev dependency, so we could generate some typed document nodes and use them in type tests, for example. But I don't think this is something our users should install.

Yes, I had concerns with this but I did not know how to approach. One thought I had was that we can duplicate the TypedDocumentNode contract into the main package code.

export interface TypedDocumentNode<Result = { [key: string]: any; }, Variables = { [key: string]: any; }> extends DocumentNode { __resultType?: Result; __variablesType?: Variables; }

Maybe add @graphql-typed-document-node/core as dev dependency for testing. What do you think of that approach? I assume TypedDocumentNode contract shouldn't change too often and an acceptable risk, but I'm just guessing.

@kettanaito I updated the code to where there isn't a dependency on @graphql-typed-document-node/core, and duplicated the contract. I figure easier to see the code. I'm not sure how to test. You would have to guide me as I'm weak in that area. I decided not to add @graphql-typed-document-node/core as a dev dependency unless we are going to test.

@kettanaito I was able to figure out and test following the pattern in place with graphql.test-d.ts and having graphql-typed-document-node/core as a dev dependency. Let me know what you think.

Hey, @longility. Those changes look fantastic.
Could you please tell me how do you generate the graphql.test-data.ts?

Of course.

Here is an example. I actually used this to generate the graphql.test-data.ts. longility@b96da41

I used this for reference: https://github.com/dotansimha/graphql-typed-document-node#how-to-use

Know that you can run against like gql tag function. In example, I'm using *.graphql file instead.

I have not done enough to know how I feel about all this yet as mentioned before. I'm experimenting. :)

The more I think about this feature, the more it sounds like a standalone usage example. I'm thinking maybe we can move this into a with-graphql-code-generator example in our examples repo, while leaving the support implementation in MSW. What are your thoughts on this? Let's discuss this before proceeding if you don't mind.

I don't mind @kettanaito . I want to consider your input more heavily as you are more familiar with the repo and ecosystem of msw than I am.

If you are talking about the codegen example commit I linked (which I used to generate graphql.test-data.ts), then yes, I think it makes sense for with-graphql-code-generator in https://github.com/mswjs/examples/tree/master/examples is what I believe you are talking about. Also, maybe an addition to the docs like somewhere here: https://mswjs.io/docs/api/graphql/query#usage-with-typescript.

For transparency, this PR does not have that codegen example commit I shared, that was another branch to help you get an idea and I didn't want to add that extra maintenance on this PR.

kettanaito

I've updated your feature branch with a few changes, let me know if they make sense.
Overall, this looks great. There's only one concern about adding @graphql-typed-document-node/core as a dependency, I don't think that's what we should do (see my reasoning in the comments).

kettanaito · 2021-07-17T11:22:47Z

I will prepare an example and add it to the examples repo. Then we can clean up this pull request not to contain @graphql-typed-document-node at all (previously needed only for testing, the newly added example will act as a test).

kettanaito · 2021-07-18T09:47:13Z

Hey, @longility. I've added a GraphQL Code Generator usage example to the examples repo and it looks good. We'd have to merge this change first for its tests to pass, and there's also a dependency update necessary on the @graphql-typed-document-node/core side to make the types compatible.

Please, would you have a chance to rebase this branch and remove everything @graphql-typed-document-node-related from it (as it now belongs in the example)? We should use DocumentNode type from the graphql package to annotate the expected selector type of the GraphQL handlers (TypedDocumentNode extends DocumentNode).

Uses DocumentNode in GraphQL tests feat: remove @graphql-typed-document-node/core and duplicate `TypedDocumentNode` interface fix: eslint issues test: TypedDocumentNode expected typings Adds positive assertions to "graphql.test-d.ts"

kettanaito · 2021-07-29T13:39:00Z

I've rebased the branch, squashing the initial implementation. Removed any external dependencies, and added an integration (interception) test to assert that MSW successfully intercepts and mocks GraphQL operations given their DocumentNode. Everything seems to be in order.

Regarding the TypedDocumentNode validation, I've prepared the usage example, but it requires the GraphQL Code Generator maintainers to update their graphql dependency (dotansimha/graphql-typed-document-node#53). Once that's done, the smoke test for the TypedDocumentNode will be merged and ready.

kettanaito

Thank you for making this happen, @longility!

kettanaito · 2021-07-29T14:45:21Z

Welcome to contributors! 🎉

longility · 2021-08-04T20:03:43Z

Thanks @kettanaito for picking up where I left off as I was not as available.

longility force-pushed the feat/support-typed-document-node branch 3 times, most recently from 452a40c to 33bd79d Compare June 9, 2021 11:09

longility commented Jun 9, 2021

View reviewed changes

kettanaito force-pushed the feat/support-typed-document-node branch 2 times, most recently from 4c9b7f2 to 3d09e18 Compare June 12, 2021 10:27

kettanaito reviewed Jun 12, 2021

View reviewed changes

src/handlers/GraphQLHandler.test.ts Show resolved Hide resolved

kettanaito reviewed Jun 12, 2021

View reviewed changes

longility changed the title ~~feat(gql): support TypedDocumentNode~~ feat(gql): support DocumentNode and TypedDocumentNode Jun 12, 2021

kettanaito added feature api:graphql labels Jun 16, 2021

kettanaito mentioned this pull request Jul 18, 2021

Add GraphQL Code Generator example mswjs/examples#59

Draft

6 tasks

kettanaito force-pushed the feat/support-typed-document-node branch 2 times, most recently from 60c4f06 to 531e8ce Compare July 29, 2021 13:14

longility and others added 2 commits July 29, 2021 15:31

feat(gql): support TypedDocumentNode

d2a4a2a

Uses DocumentNode in GraphQL tests feat: remove @graphql-typed-document-node/core and duplicate `TypedDocumentNode` interface fix: eslint issues test: TypedDocumentNode expected typings Adds positive assertions to "graphql.test-d.ts"

GraphQL: Adds integration test for the "DocumentNode" as input

f473514

kettanaito force-pushed the feat/support-typed-document-node branch from 531e8ce to f473514 Compare July 29, 2021 13:32

kettanaito approved these changes Jul 29, 2021

View reviewed changes

kettanaito merged commit 8f77f4c into mswjs:master Jul 29, 2021

longility deleted the feat/support-typed-document-node branch August 4, 2021 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gql): support DocumentNode and TypedDocumentNode #774

feat(gql): support DocumentNode and TypedDocumentNode #774

longility commented Jun 8, 2021

codesandbox-ci bot commented Jun 8, 2021 •

edited

Loading

longility Jun 9, 2021

longility Jun 9, 2021

kettanaito Jun 12, 2021

longility commented Jun 9, 2021

kettanaito commented Jun 12, 2021

kettanaito Jun 12, 2021

kettanaito Jun 12, 2021

kettanaito Jun 12, 2021

longility Jun 12, 2021

longility Jun 12, 2021

longility Jun 12, 2021

kettanaito Jun 14, 2021

longility Jun 14, 2021

kettanaito Jun 14, 2021

longility Jun 14, 2021

kettanaito left a comment

kettanaito commented Jul 17, 2021

kettanaito commented Jul 18, 2021

kettanaito commented Jul 29, 2021

kettanaito left a comment

kettanaito commented Jul 29, 2021

longility commented Aug 4, 2021

feat(gql): support DocumentNode and TypedDocumentNode #774

feat(gql): support DocumentNode and TypedDocumentNode #774

Conversation

longility commented Jun 8, 2021

codesandbox-ci bot commented Jun 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longility commented Jun 9, 2021

kettanaito commented Jun 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kettanaito left a comment

Choose a reason for hiding this comment

kettanaito commented Jul 17, 2021

kettanaito commented Jul 18, 2021

kettanaito commented Jul 29, 2021

kettanaito left a comment

Choose a reason for hiding this comment

kettanaito commented Jul 29, 2021

longility commented Aug 4, 2021

codesandbox-ci bot commented Jun 8, 2021 •

edited

Loading