Skip to content

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

Open
ya-s-u wants to merge 1 commit into
Shopify:v1.x-2022-07from
ya-s-u:typed-document-node
Open

Add support for graphql-typed-document-node#2353
ya-s-u wants to merge 1 commit into
Shopify:v1.x-2022-07from
ya-s-u:typed-document-node

Conversation

@ya-s-u

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

Copy link
Copy Markdown
Contributor

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

@frehner frehner left a comment

Copy link
Copy Markdown
Contributor

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