Skip to content
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

maskFragments for nested fragments forces weird nesting to resolve types #410

Closed
3 tasks done
AlexanderArvidsson opened this issue Oct 4, 2024 · 1 comment
Closed
3 tasks done

Comments

@AlexanderArvidsson
Copy link

AlexanderArvidsson commented Oct 4, 2024

Describe the bug

Building on the example example-pokemon-api with maskFragments by using a nested fragment and trying to mask some existing data to the fragments, you're forced to do some weird nesting to get it to work.
The documentation doesn't cover this case, so I wonder if the below is the intended way of doing it.

const PokemonNameFragment = graphql(`
  fragment PokemonName on Pokemon {
    name
  }
`);

export const PokemonItemFragment = graphql(
  `
    fragment PokemonItem on Pokemon {
      id
      ...PokemonName
    }
  `,
  [PokemonNameFragment]
);

const data = maskFragments([PokemonItemFragment, PokemonNameFragment], {
  id: '001',
  name: 'Bulbasaur',
  ...maskFragments([PokemonNameFragment], {
    name: 'Bulbasaur',
  }),
});

const a = readFragment(PokemonItemFragment, data);
const b = readFragment(PokemonNameFragment, data);
console.log(a.id, b.name)

If you don't spread the nested maskFragments, you get a type error:

Diagnostics:
1. Argument of type '{ id: string; name: string; }' is not assignable to parameter of type '{ name: string; } & { id: string; [$tada.fragmentRefs]: { PokemonName: "Pokemon"; }; }'.
     Property '[fragmentRefs]' is missing in type '{ id: string; name: string; }' but required in type '{ id: string; [$tada.fragmentRefs]: { PokemonName: "Pokemon"; }; }'. (ts) [2345] [2345]

I would expect to be able to just do the following:

const data = maskFragments([PokemonItemFragment, PokemonNameFragment], {
  id: '001',
  name: 'Bulbasaur',
});

If, however, the spreading is the intended way to do this, I suggest instead updating the docs to have some notes about this, and also the fact that you must duplicate your fields in the inner and outer data object.

Reproduction

Modified version of the example, just place the above code in examples/example-pokemon-api/src/components/PokemonItem.tsx
https://github.com/0no-co/gql.tada/tree/main/examples/example-pokemon-api

gql.tada version

1.8.10

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@AlexanderArvidsson AlexanderArvidsson changed the title Type of maskFragments for nested fragments forces weird nesting to resolve types maskFragments for nested fragments forces weird nesting to resolve types Oct 4, 2024
@kitten
Copy link
Member

kitten commented Oct 6, 2024

This is basically an impossible condition, and it's not explained in the docs, since, in theory the type error is showing the issue quite clearly. Let's break it down 👀

  • { id: string; name: string; } is what's provided
  • the PokemonItem fragment spreads the PokemonName fragment
  • this means that the provided type is matched against PokemonItem
    • importantly: the type isn't yet matched against PokemonName, since that's in a nested condition/spread
  • this is basically then communicated in the output type as:
    • [$tada.fragmentRefs]: { PokemonName: "Pokemon"; }

In other words, since the nested fragment spread is the important point here.
Since there's a nested spread and maskFragments resolves one level of masking, the next level of masking cannot be unwrapped, otherwise you're in the territory of what unsafe_readResult is supposed to do.

Since this confusion therefore overlaps with a desire to use something more like unsafe_readResult (but presumably while keeping type safety, let's take this discussion over there and close this as a duplicate:
#273

Happy to reopen this of course, I'm just pretty convinced the main issue here is the lack of what's described in the other issue, for which we haven't come up with a solution yet that fulfills all requirements basically

@kitten kitten closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2024
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

No branches or pull requests

2 participants