-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: always inline and deduplicate fragment definitions #8971
Conversation
🦋 Changeset detectedLatest commit: 91479c5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
} | ||
|
||
let meta: ExecutableDocumentNodeMeta | void; | ||
const jsonStringify = (json: unknown) => | ||
JSON.stringify(json, (key, value) => (key === 'loc' ? undefined : value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loc is only bloat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
@@ -1304,7 +1304,7 @@ export * from "./gql.js";`); | |||
|
|||
export type CFragment = { __typename?: 'Query', c?: string | null } & { ' $fragmentName'?: 'CFragment' }; | |||
|
|||
export const CFragmentDoc = {} as unknown as DocumentNode<CFragment, unknown>; | |||
export const CFragmentDoc = {"kind":"Document","definitions":[{"kind":"FragmentDefinition","name":{"kind":"Name","value":"C"},"typeCondition":{"kind":"NamedType","name":{"kind":"Name","value":"Query"}},"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"c"}}]}}]} as unknown as DocumentNode<CFragment, unknown>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this can actually be used with readFragment and stuff 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing!
🚀 Snapshot Release (
|
Package | Version | Info |
---|---|---|
@graphql-cli/codegen |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/cli |
3.1.0-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/core |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/add |
4.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/fragment-matcher |
4.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/introspection |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/schema-ast |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/visitor-plugin-common |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-document-nodes |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/gql-tag-operations |
2.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-operations |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-resolvers |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typed-document-node |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/client-preset |
2.1.0-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/gql-tag-operations-preset |
2.1.0-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/graphql-modules-preset |
3.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/testing |
2.0.1-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/plugin-helpers |
4.1.0-alpha-20230215090240-91479c550 |
npm ↗︎ unpkg ↗︎ |
🚀 Website PreviewThe latest changes to the website are available as preview in: https://2ff613ab.graphql-code-generator.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's mark dedupeFragments
as a deprecated option so we can drop it in the next major.
2b836fc
to
233ac86
Compare
This is a breaking change IMO and should only be done in a major version. Our generated file size just jumped from 600kb to a whopping 3.7MB causing issues on our frontend since there is a lot of fragment reuse going on (150 queries, lots and lots of fragments). Our Lighthouse score totally got trashed since we shipped a gigantic file, please reconsider 🙏 or keep the option in there like it was before. We are using this in combination of typed-document-node, to be honest we never had issues with duplicate fragments (we are not using client-present atm). Regards, |
@JanStevens Please use the SWC or babel plugin. Also, you can use persisted documents and completely drop the AST/string being shipped to the client. https://the-guild.dev/graphql/codegen/plugins/presets/preset-client#persisted-documents |
I know about persisted documents but this is not implemented everywhere. For example when querying headless CMS like Contentful or DatoCMS, they don't offer a persisted query implementation. The only option is to add your own graphql server on top of their existing implementation, which is not really maintainable. |
You can also minify and transform your documents (e.g. inline fragments). See |
The option
dedupeFragments
has been problematic from the beginning and caused a lot of unexpected behavior that lead to either duplicated fragment definitions within a document node or missing fragment definitions within a document node.ATTENTION !!! This PR basically replaces the existing definitions spread (
...
) with a duplication/inline strategy. While this increases the bundle size of the client applications, it is actually the only possible way of solving this issue once and for all.I marked this change as a fix, though it might be debatable that this should instead be a breaking change, which we could also ship IMHO.
Closes #8967
Closes #8842