Add TypedDocumentNode string alternative #9137

beerose · 2023-03-08T15:44:00Z

🚨 Depends on a PR in graphql-typed-document-node

Description

Related to: #8296:

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take action on it as appropriate

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Test A
Test B

Test Environment:

OS:
@graphql-codegen/...:
NodeJS:

Checklist:

I have followed the CONTRIBUTING doc and the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

github-actions · 2023-03-08T15:46:34Z

🚀 Snapshot Release (`alpha`)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package	Version	Info
`@graphql-codegen/cli`	`3.3.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/visitor-plugin-common`	`3.1.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-document-nodes`	`3.0.3-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/gql-tag-operations`	`3.0.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-operations`	`3.0.3-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-resolvers`	`3.2.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typed-document-node`	`4.0.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript`	`3.0.3-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/client-preset`	`3.0.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/gql-tag-operations-preset`	`3.0.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/graphql-modules-preset`	`3.1.2-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/plugin-helpers`	`4.2.0-alpha-20230321125704-a86f4bcb9`	npm ↗︎ unpkg ↗︎

changeset-bot · 2023-03-08T15:47:27Z

🦋 Changeset detected

Latest commit: a86f4bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@graphql-codegen/client-preset	Major
@graphql-codegen/typed-document-node	Major
@graphql-codegen/gql-tag-operations	Major
@graphql-codegen/gql-tag-operations-preset	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-03-08T15:52:51Z

💻 Website Preview

The latest changes are available as preview in: https://f9b2e87e.graphql-code-generator.pages.dev

beerose · 2023-03-09T10:52:28Z

examples/react/urql-string-document-mode/.gitignore

@@ -0,0 +1,23 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.


Not sure if we want a new example or just modify an existing one?

I think urql is not a good example because it actually requires the AST at runtime anyways 🤔 .
We can add or modify a react-query or similar example that does not require the AST.
What do you think?

I think urql is not a good example because it actually requires the AST at runtime anyways

Does it, though? It accepts both string and AST and seems to work fine (I modified the original urql example). Unless I'm missing something.

Regarding the react-query example — it currently uses @graphql-tools/executor-http, but I could replace it with fetch.

yeah, urql uses graphql internally and thus will just parse the string to the ast at runtime

yeah, I am fine with replacing executor HTTP in react-query!

beerose · 2023-03-09T10:53:16Z

.changeset/@graphql-codegen_client-preset-9137-dependencies.md

+dependencies updates:
+  - Updated dependency [`@graphql-typed-document-node/core@3.2.0-alpha-20230309104331-975d9d2` ↗︎](https://www.npmjs.com/package/@graphql-typed-document-node/core/v/3.2.0) (from `3.1.2`, in `dependencies`)


TODO: update with the latest version when a PR to @graphql-typed-document-node/core is accepted and released

beerose · 2023-03-09T10:53:39Z

packages/plugins/typescript/typed-document-node/src/index.ts

-  if (config && config.documentMode === DocumentMode.string) {
-    throw new Error(`Plugin "typed-document-node" does not allow using 'documentMode: string' configuration!`);
-  }


Not sure about this

beerose · 2023-03-09T10:53:59Z

packages/presets/client/package.json

@@ -26,7 +26,7 @@
    "@graphql-codegen/gql-tag-operations": "2.0.2",
    "@graphql-codegen/plugin-helpers": "^4.1.0",
    "@graphql-codegen/visitor-plugin-common": "^3.0.2",
-    "@graphql-typed-document-node/core": "3.1.2",
+    "@graphql-typed-document-node/core": "3.2.0-alpha-20230309104331-975d9d2",


packages/presets/client/src/fragment-masking-plugin.ts

examples/react/urql-string-document-mode/src/App.tsx

n1ru4l · 2023-03-13T15:40:02Z

I think we will also have issues with metadata within the string document mode 🤔 #8757

Any idea how we could still support metadata with strings? Maybe instead we should export an object with a string (operations string key) and a meta object?

Also I saw that we embed fragments with ... here, would this cause the same issues as mentioned/fixed by #8971 for document nodes?

beerose · 2023-03-13T18:08:56Z

Also I saw that we embed fragments with ... here, would this cause the same issues as mentioned/fixed by #8971 for document nodes?

Done ~~Yeah, good point. Will work on that.~~

I think we will also have issues with metadata within the string document mode 🤔 #8757

Yep. Will work on that too. Exporting an object sounds good — I don't really have other ideas right now except for class MyString extends String which seems a bit ugly 😅

Edit: after thinking about it for a bit, I realised that the class may be a good solution here (even if ugly 😅). I'm thinking about something like this:

import { DocumentTypeDecoration } from '@graphql-typed-document-node/core';

export type TypedDocumentString<TResult, TVariables> = DocumentTypeDecoration<TResult, TVariables> & string;

class DocumentStringWithMeta<TResult, TVariables>
  extends String
  implements DocumentTypeDecoration<TResult, TVariables>
{
  __apiType?: DocumentTypeDecoration<TResult, TVariables>['__apiType'];

  constructor(value: string, public __meta__: { hash: string }) {
    super(value);
  }
}

const _a = new DocumentStringWithMeta(`
  query {
    ids
  }
`,
  { hash: '123' }
);

const meta = _a.__meta__.hash;

Now a question is — should we only use DocumentStringWithMeta with persisted documents, or maybe we always want to use this (and hence kinda advertising persistent documents as a default practice)?

Also, I'm not sure if @graphql-typed-document-node/core is the best place for that — maybe we can inline this class in the generated file?

Are there any other things like that (things that would cause problems with string documents)? I guess it would be a good idea to extend the tests (or some of them) to also run in "string" document mode. What do you think, @n1ru4l?

github-actions · 2023-03-15T14:39:33Z

diff --git a/website/algolia-lockfile.json b/website/algolia-lockfile.json
index 86c85dfcb..9596c78cf 100644
--- a/website/algolia-lockfile.json
+++ b/website/algolia-lockfile.json
@@ -2043,7 +2043,8 @@
       "Config API",
       "Fragment Masking",
       "Persisted documents",
-      "Reducing bundle size: Babel plugin"
+      "Reducing bundle size: Babel plugin",
+      "DocumentMode"
     ],
     "toc": [
       {
@@ -2109,6 +2110,17 @@
         ],
         "title": "Reducing bundle size: Babel plugin",
         "anchor": "reducing-bundle-size-babel-plugin"
+      },
+      {
+        "children": [
+          {
+            "children": [],
+            "title": "When to use a string DocumentMode?",
+            "anchor": "when-to-use-a-string-documentmode"
+          }
+        ],
+        "title": "DocumentMode",
+        "anchor": "documentmode"
       }
     ],
     "content": "d41d8cd98f00b204e9800998ecf8427e",

website/src/pages/plugins/presets/preset-client.mdx

examples/persisted-documents/package.json

beerose · 2023-03-20T13:34:09Z

dev-test/gql-tag-operations/graphql/fragment-masking.ts

-export type FragmentType<TDocumentType extends DocumentNode<any, any>> = TDocumentType extends DocumentNode<
-  infer TType,
-  any
->
-  ? TType extends { ' $fragmentName'?: infer TKey }
-    ? TKey extends string
-      ? { ' $fragmentRefs'?: { [key in TKey]: TType } }
+export type FragmentType<TDocumentType extends DocumentTypeDecoration<any, any>> =
+  TDocumentType extends DocumentTypeDecoration<infer TType, any>
+    ? TType extends { ' $fragmentName'?: infer TKey }
+      ? TKey extends string
+        ? { ' $fragmentRefs'?: { [key in TKey]: TType } }
+        : never


Is this considered a breaking change for the codegen or not? @n1ru4l @saihaj

I don't think so, people shouldn't use this type with anything else than our generated stuff.

agreed should be fine

Okay, as I think about it a bit more, this change kinda requires people to upgrade their typed document node version. 🤔

In graphql-yoga we treat peerDependency upgrades depending on the type of change.

e.g.
X.1.1 bump of a peer dependency would be a breaking change
1.X.1 bump of a peer dependency would be a minor change
1.1.X bump of a peer dependency would be a patch change

So I think we should treat this the same here (minor change).

Here the special case is that it is not listed as a peer dependency, but theoretically, it could be one, as people will need to install it, otherwise, their code won't compile with strict package manages like pnpm and yarn that do no hoisting. 🤔

Then, however, whether it should be a peer dependency or not also depends on the location where the code is generated.

So the safest option would be to ship this as a breaking change. I am sure people will open issues if we don't...

beerose added 2 commits March 8, 2023 13:51

Add unit tests

4a7b339

Add TypedDocumentNode string alternative

504367a

beerose and others added 2 commits March 9, 2023 11:51

Temporarily use snapshot version of @graphql-typed-document-node/core

c7105db

chore(dependencies): updated changesets for modified dependencies

13e48a9

beerose commented Mar 9, 2023

View reviewed changes

packages/presets/client/src/fragment-masking-plugin.ts Show resolved Hide resolved

beerose added 3 commits March 9, 2023 11:59

Update fragment-masking.ts files

02ed2b3

Update fragment-masking in dev-test dir

2d5a4e7

Update @graphql-typed-document-node/core version in examples

590b2e2

beerose mentioned this pull request Mar 9, 2023

Add DocumentTypeDecoration interface with __apiType key dotansimha/graphql-typed-document-node#154

Merged

Merge branch 'master' into typed-document-string

8e4e93f

beerose commented Mar 9, 2023

View reviewed changes

examples/react/urql-string-document-mode/src/App.tsx Outdated Show resolved Hide resolved

Update test after latest implememntation changes

7a5d42d

beerose force-pushed the typed-document-string branch from 44d23aa to 7a5d42d Compare March 9, 2023 14:44

beerose added 2 commits March 10, 2023 12:33

Changeset

3892ba2

Modify existing example

ef28588

beerose marked this pull request as ready for review March 13, 2023 10:32

beerose force-pushed the typed-document-string branch from 7a94464 to 4ca4b2d Compare March 13, 2023 10:40

Update typescript-graphql-request example

961251f

beerose force-pushed the typed-document-string branch from 9bdcd81 to 961251f Compare March 13, 2023 10:53

Merge branch 'master' into typed-document-string

dbd2053

Add failing test for duplicated fragments

8005ea3

beerose requested a review from n1ru4l March 14, 2023 16:30

Merge branch 'master' into typed-document-string

8b5ad73

beerose force-pushed the typed-document-string branch from 45e26e6 to 8b5ad73 Compare March 15, 2023 11:37

github-actions bot and others added 3 commits March 15, 2023 11:38

chore(dependencies): updated changesets for modified dependencies

f960ccf

Update react-query example — use fetch insetad of http executor

ed34330

Merge branch 'master' into typed-document-string

0283daf

beerose requested review from dimaMachina and saihaj March 15, 2023 13:02

Add docs

cfd9380

beerose commented Mar 15, 2023

View reviewed changes

website/src/pages/plugins/presets/preset-client.mdx Outdated Show resolved Hide resolved

saihaj reviewed Mar 16, 2023

View reviewed changes

examples/persisted-documents/package.json Outdated Show resolved Hide resolved

beerose and others added 3 commits March 20, 2023 12:05

Update @graphql-typed-document-node/code version

a97276b

chore(dependencies): updated changesets for modified dependencies

2e99784

Add more info to the dosc

bdc1e7d

beerose force-pushed the typed-document-string branch from 00d5746 to bdc1e7d Compare March 20, 2023 11:37

Merge branch 'master' into typed-document-string

e954bea

beerose requested a review from saihaj March 20, 2023 11:39

saihaj approved these changes Mar 20, 2023

View reviewed changes

beerose commented Mar 20, 2023

View reviewed changes

n1ru4l approved these changes Mar 21, 2023

View reviewed changes

beerose added 4 commits March 21, 2023 12:16

Update docs

0f155b4

Update changeset

5771e6d

Update versions in changeset

5ce6b4f

Merge branch 'master' into typed-document-string

a86f4bc

beerose merged commit 2256c8b into master Mar 21, 2023

beerose deleted the typed-document-string branch March 21, 2023 13:41

theguild-bot mentioned this pull request Mar 21, 2023

Upcoming Release Changes #9132

Merged

glenjamin mentioned this pull request May 25, 2023

toString on TypedDocumentNode dotansimha/graphql-typed-document-node#156

Closed

benasher44 mentioned this pull request Jun 3, 2023

Support TypedDocumentString via DocumentTypeDecoration in @apollo/client apollographql/apollo-feature-requests#390

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TypedDocumentNode string alternative #9137

Add TypedDocumentNode string alternative #9137

beerose commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

changeset-bot bot commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

beerose Mar 9, 2023

n1ru4l Mar 13, 2023

beerose Mar 13, 2023

n1ru4l Mar 14, 2023

n1ru4l Mar 14, 2023

beerose Mar 9, 2023

beerose Mar 9, 2023

beerose Mar 9, 2023

n1ru4l commented Mar 13, 2023

beerose commented Mar 13, 2023 •

edited

Loading

github-actions bot commented Mar 15, 2023 •

edited

Loading

beerose Mar 20, 2023

n1ru4l Mar 20, 2023

saihaj Mar 21, 2023

n1ru4l Mar 21, 2023

		@@ -0,0 +1,23 @@
		# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.

		dependencies updates:
		- Updated dependency [`@graphql-typed-document-node/core@3.2.0-alpha-20230309104331-975d9d2` ↗︎](https://www.npmjs.com/package/@graphql-typed-document-node/core/v/3.2.0) (from `3.1.2`, in `dependencies`)

Add TypedDocumentNode string alternative #9137

Add TypedDocumentNode string alternative #9137

Conversation

beerose commented Mar 8, 2023 • edited Loading

Description

Type of change

Screenshots/Sandbox (if appropriate/relevant):

How Has This Been Tested?

Checklist:

Further comments

github-actions bot commented Mar 8, 2023 • edited Loading

🚀 Snapshot Release (alpha)

changeset-bot bot commented Mar 8, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 8, 2023 • edited Loading

💻 Website Preview

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

n1ru4l commented Mar 13, 2023

beerose commented Mar 13, 2023 • edited Loading

github-actions bot commented Mar 15, 2023 • edited Loading

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

beerose commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

🚀 Snapshot Release (`alpha`)

changeset-bot bot commented Mar 8, 2023 •

edited

Loading

github-actions bot commented Mar 8, 2023 •

edited

Loading

beerose commented Mar 13, 2023 •

edited

Loading

github-actions bot commented Mar 15, 2023 •

edited

Loading