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

Add support for graphql-typed-document-node #2353

Open
wants to merge 1 commit into
base: v1.x-2022-07
Choose a base branch
from

Conversation

ya-s-u
Copy link
Contributor

@ya-s-u ya-s-u commented Nov 27, 2022

Description

Related #200

I propose to support typed-document-node for more typesafety.
https://github.com/dotansimha/graphql-typed-document-node

Additional context

  • Need to add a test?

Before submitting the PR, please make sure you do the following:

  • Read the Contributing Guidelines
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123)
  • Update docs in this repository according to your change
  • Run yarn changeset add if this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning

@ya-s-u ya-s-u changed the title Add support for typed-document-node Add support for graphql-typed-document-node Nov 27, 2022
Copy link
Contributor

@frehner frehner left a comment

Choose a reason for hiding this comment

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

Hello, thanks for this! We're actually in the process of working on Hydrogen v2, and I've been looking into adding support for typed-document-node in that version. I would love to get your eyes on the v2 implementation if that's ok.

A related question / issue I have, and would love your thoughts or input on: in the past, we have found performance issues with using the GraphQL Documents at runtime, and performance greatly improved when we got rid of the documents (and the document parsing) and just used the raw GraphQL strings.

So I'm a little concerned about the runtime performance implications here of using the print() command from GraphQL. However, it could also be that the performance issues we ran into were caused by creating the Documents, and not by parsing them.

With the client preset of GraphQL codegen, it creates the Documents at build time and not during runtime, so maybe that's ok (or maybe it's not?).

What are your thoughts on the performance implications of this? Also, is there any way to update the client codegen preset to output strings instead of documents so we don't have to use print() at all?

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