search

apollographql / graphql-testing-library

Testing utilities that encourage good practices for apps built with GraphQL.
https://apollographql.github.io/graphql-testing-library/
MIT License
37 stars 1 forks source link

Code feedback #44

Open kettanaito opened 3 months ago

kettanaito commented 3 months ago

Hey! So excited to finally see this library out!

I will provide a few code review and suggestions below. Feel free to convert this to a discussion or whichever format is more suitable for you.

https://github.com/apollographql/graphql-testing-library/blob/e18caf4ef4b3133b438500bafebdf127e4189417/src/handlers.ts#L92-L94

This isn't something MSW can solve with the current version range of TS we support, unfortunately. You can learn more about why here: https://github.com/mswjs/msw/issues/1691.

Solution: Provide a union of expected response types to the handler. I'm not sure if graphql.*() functions support that right now though. It looks like they infer the response resolver (and its return type) from the query. Let me know if this turns problematic, we can work on a solution for it.

https://github.com/apollographql/graphql-testing-library/blob/e18caf4ef4b3133b438500bafebdf127e4189417/src/handlers.ts#L163-L164

It would be great to let the folks use the built-in delay values too, like infinite or random server response time. Looks like a change here.

For debugging reasons, it would be nice to see what schema was used when a handler was run. You can achieve that by creating a custom request handler instead of wrapping graphql.operation(). You can then define its log() method to include the schema when debugging handlers.

I think that can be handy given you can swap the schema mid-test. You also get access to internal caching API which you can use to cache any computation you need (like gql(query) or AST parsing, if that makes sense).

If you have any questions about the WebSocket mocking, please let me know.

alessbell commented 3 months ago

Hey @kettanaito 👋

Thanks so much for taking the time to dig in and for your comments! This is great feedback - I'll keep this open as I work through these items. I'm working on WS mocking right now and appreciate the offer to field any questions - I'll let you know, and will otherwise link to the PR once it's up :)

alessbell commented 2 months ago

One update on using the built-in delay values, something I recently shipped. It's definitely nice to be able to reuse that :)

I noticed flakiness in Jest tests with concurrent React autobatching in a scenario with GraphQL queries with @defer and multipart responses. With a 5ms delay between chunks, they were sometimes being batched into a single render.

I want the default behavior in Node processes to be "each chunk causes a re-render" so I upped it to 20ms which seems to be firmly on the "not autobatched" side of the chasm: https://github.com/apollographql/graphql-testing-library/blob/main/src/handlers.ts#L41-L47 though I still need to test this with React 19 RCs.

Finally I'm continuing work on subscriptions, so far so good!

kettanaito commented 2 months ago

So excited to hear how the subscriptions will turn out!

A bit updates from my end, I'm planning on releasing the WebSocket support in MSW later this month. @alessbell, would I be able to hear your feedback before that? I don't mind postponing, it's really about hearing back and shipping the best API possible.

alessbell commented 2 months ago

@kettanaito that's very exciting! So far I haven't run into any issues - I'm working out some details around reconnection attempts, but I'm far enough along that I'd say it's all clear on my end :)