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

fix: add support for TypedDocumentNode to be passed to WS methods #585

Merged
merged 1 commit into from
Sep 27, 2023
Merged

fix: add support for TypedDocumentNode to be passed to WS methods #585

merged 1 commit into from
Sep 27, 2023

Conversation

crutchcorn
Copy link
Contributor

This PR adds support for TypedDocumentNodes to be passed to the subscribe and request methods of the WebSockets implementation.

This allows the return type from next and variables to be properly typed when paired with @graphql-codegen/cli

I have tested this change in my codebase and it appears to function as I would expect on the type level

Copy link
Member

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

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

Thanks!

@crutchcorn
Copy link
Contributor Author

Oh, shoot - format seems to be broken. Apologies - I ran prettier but I may have missed a format script. Let me look for that and make a commit.

@crutchcorn
Copy link
Contributor Author

image

Hmm, I seem to see that there's a few files that need to be formatted that aren't related to my change. Should I commit them or leave it for a different PR? (which I'm happy to open)

@jasonkuhrt
Copy link
Member

@crutchcorn a format would be welcome, that is from having merged some updates to prettier but I didn't re-format trunk yet.

@jasonkuhrt
Copy link
Member

@crutchcorn Actually it's fine, nothing to do with you or this PR :)

@jasonkuhrt jasonkuhrt merged commit a9fcb44 into graffle-js:main Sep 27, 2023
5 of 7 checks passed
@crutchcorn crutchcorn deleted the add-typed-document-node-to-ws branch September 27, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants