Allow FragmentType to be called as FragmentType<TypedDocumentNode>#13009
Allow FragmentType to be called as FragmentType<TypedDocumentNode>#13009phryneas merged 7 commits intorelease-4.1from
FragmentType to be called as FragmentType<TypedDocumentNode>#13009Conversation
…but also as `FragmentType<TypedDocumentNode>`.
🦋 Changeset detectedLatest commit: a5ee13a The changes in this PR will be included in the next version bump. 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 |
commit: |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the FragmentType utility type to accept not only fragment data types (TData) but also TypedDocumentNode instances. This allows users to pass either the fragment type directly or the type of a TypedDocumentNode variable, and the type system will correctly extract the fragment data.
Key changes:
- Modified
FragmentTypeto use conditional type checking withDocumentTypeDecorationto handle both usage patterns - Added comprehensive test coverage for the new functionality
- Updated API reports to reflect the new type signature
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/masking/types.ts | Enhanced FragmentType with conditional type logic to accept both TData and TypedDocumentNode parameters |
| src/masking/benches/types.bench.ts | Added test cases for the new FragmentType usage patterns and updated instantiation count |
| .changeset/few-parrots-raise.md | Added changeset documenting the new feature |
| .api-reports/api-report.api.md | Updated API report with new FragmentType signature |
| .api-reports/api-report-masking.api.md | Updated masking API report with new FragmentType signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 12 changed, 0 removedBuild ID: 7e5f4f94d934ad7d0d185b66 |
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
Looks good! Only had one question about why you chose one type vs another
src/masking/types.ts
Outdated
| import type { TypeOverrides } from "@apollo/client"; | ||
| import type { ApplyHKTImplementationWithDefault } from "@apollo/client/utilities/internal"; | ||
|
|
||
| import type { DocumentTypeDecoration } from "@graphql-typed-document-node/core"; |
There was a problem hiding this comment.
For my own education, why use DocumentTypeDecoration here instead of TypedDocumentNode?
There was a problem hiding this comment.
It's the "typed part" of TypedDocumentNode and what the typed-document-node helpers themselves use:
export type ResultOf<T> = T extends DocumentTypeDecoration<infer ResultType, infer VariablesType> ? ResultType : never;It already works with TypedDocumentString too, and will likely work with all future similar additions to the family - since we don't explicitly rely on a Document here it just seems more future-proof.
#13009) * Allow `FragmentType` not only to be called as `FragmentType<TData>`, but also as `FragmentType<TypedDocumentNode>`. * update non-type-import * simplify type * chores * fixup 🤦 * lint * Clean up Prettier, Size-limit, and Api-Extractor --------- Co-authored-by: phryneas <[email protected]>
No description provided.