Skip to content

Implement fragment unmasking utility types #9380

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

charpeni
Copy link
Contributor

@charpeni charpeni commented May 9, 2023

Description

Implementation of fragment unmasking to support an incremental adoption of fragment masking or utils for tests.

When you enable Fragment Masking, all fragments and operations are automatically masked, even though not all fragments or operations use Fragment Masking.

We can expose the following utility types: UnmaskResultOf<TypedDocumentNode> & UnmaskFragmentType<TType>.

It checks whether we're inside an operation or directly within a fragment and then recursively flattens fragments by resolving $fragmentRefs and merging them to the root fragment.

const FilmItemFragment = graphql(`
  fragment FilmItem on Film {
    id
    title
  }
`);

const AllFilmsFragment = graphql(`
  fragment AllFilms on Root {
    allFilms {
      films {
        ...FilmItem
      }
    }
  }
`);

const myQuery = graphql(`
  query Films {
    ...AllFilms
  }
`); // DocumentTypeDecoration<R, V>

// Fragment references are all inlined rather than referring to ` $fragmentRefs`.
type UnmaskedData = UnmaskResultOf<typeof myQuery>;
//   ^? type UnmaskedData = {
//        __typename: 'Root' | undefined,
//        allFilms: {
//          films: {
//            __typename?: "Film" | undefined;
//            id: string;
//            title?: string | null | undefined;
//          }[] | null | undefined
//        } | null | undefined
//      }

Related #9075


I also introduced expect-type to test those newly generated TypeScript utilities to prevent regressions. Currently, there's no way to test it before generating fragment-masking.ts as definitions are living within strings, so we need to generate examples and then rely on a Jest test within apollo-client's example combined with expect-type to test them.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Screenshots/Sandbox (if appropriate/relevant):

Screenshot 2023-05-09 at 9 32 23 AM Screenshot 2023-06-05 at 3 42 09 PM

How Has This Been Tested?

  • Added tests to cover TypeScript utility types
  • TypeScript Playground
  • 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

@changeset-bot
Copy link

changeset-bot bot commented May 9, 2023

🦋 Changeset detected

Latest commit: f18151a

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

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/client-preset Minor

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

@charpeni charpeni force-pushed the implement-fragment-unmasking branch from 6667693 to e8152b4 Compare May 9, 2023 15:36
@charpeni charpeni changed the title Implement fragment unmasking Implement fragment unmasking utility types May 9, 2023
@Urigo Urigo requested a review from n1ru4l May 14, 2023 17:33
@charpeni charpeni force-pushed the implement-fragment-unmasking branch 2 times, most recently from a37e437 to 6c2dba5 Compare June 5, 2023 19:12
@charpeni charpeni marked this pull request as ready for review June 5, 2023 19:58
@brandonwestcott
Copy link

@charpeni just wanted to provide feedback as our team attempted to utilize these proposed types in our application. In our case, any fragment over 2 to 3 levels deep caused widespread TS2589: Type instantiation is excessively deep and possibly infinite..

@charpeni
Copy link
Contributor Author

@brandonwestcott Do you believe it's something that could be reproduced on TypeScript Playground?

Not directly related to GraphQL Code Generator, but I noticed some weird behaviors starting with TypeScript 5.0 and worse with 5.1 related to Type instantiation is excessively deep and possibly infinite when it's not excessively deep at all.

I tried to reproduce it on TypeScript Playground with one query and five nested fragments, and I couldn't: 🔗 TypeScript Playground.

But! Weirdly enough, I was able to reproduce it when trying to unmask a type that couldn't be resolved: 🔗 TypeScript Playground.

Screenshot 2023-06-23 at 2 43 24 PM

@MatthiasvB
Copy link

MatthiasvB commented Jul 12, 2023

I like the idea a lot. But here's my problem: I'm in Vue, using bindings from @urql/vue (useQuery, useFragment and so on). Since the types proposed here accept as input the result from a graphql('...') function call, I'd have to

const rawQuery = graphql(`...`);
const queryResData = useQuery({
  query: rawQuery
}).data as UnmaskResultOf<typeof rawQuery>

which is quite entangled. I'd love to be able to write a function unmaskAllFragments, which would be a no-op only doing the necessary assertion. To be used like

const queryRes = unmaskAllFragments(useQuery({
  query: graphql(`...`)
});

This would require the generic type accepted by UnmaskResultOf to be something other than DocumentTypeDecoration, because that is no longer present in the result of useQuery (but, all the necessary information should still be there, since I can "manually" unmask the fragment with useFragment).

I'll dig around a bit more, maybe I'll find a way to achieve this, but I wouldn't bet on it ;)

@bryanmylee
Copy link

Just to note, the utility type doesn't work on fragments applied to nested objects of the query, which my team unfortunately depends on.

Playground link.

@bryanmylee
Copy link

bryanmylee commented Aug 1, 2023

Adapted your implementation to take it one step further, and I've tested for:

  1. nested fragments / sibling fragments
  2. fragments in nested objects
  3. fragments in arrays
  4. fragments in optional fields

The implementation can be found here.

@tnyo43
Copy link
Contributor

tnyo43 commented Oct 12, 2023

No more updates?
I would like to use this utility type too. I will try to implement it in another PR if there is no further update

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.

5 participants