Data masking with fragment that should never be masked

[Cellule]

Moved from #11666 (comment)

What happens if a fragment is expected to NOT be masked, always?
I have some fragment on types that don't have clear ids, so I can't useFragment on it.

So far I've been using useFragment from GraphQL Codegen client preset (which I renamed unmaskFragmentData) in the component using the fragment
In that case it is expected that, while the typings are masked, the data is available at runtime to be unmasked.

I can go put @unmask everywhere that fragment is spread, but that's error-prone.
I'm fiddling with a custom directive @disableMasking on the FragmentDefinition to ensure all places using that fragment are using @unmask and are not using @nonreactive
Now I'm hitting a typings issue because it unmasks the fragment, effectively making all the data available top-level, but I really only want it to be available on the component.

How can I make sure that the data will be available at runtime, but is masked in the typings?
I basically want @unmask at runtime, but it shouldn't be there for typings codegen

Right now, my approach is to enforce the @unmask directive for those fragment spreads and during codegen I strip them before running typescript-operations and leave them for the runtime (at some point because I'm not ready to try apollo data masking)

Note that I'm working with a heavily customized fork GraphQL Codegen client preset to make it work with Apollo client more natively

[jerelmiller]

@Cellule if you're using GraphQL Codegen, it should keep those types masked. Support for Apollo's @unmask directive in GraphQL Codegen landed in typescript-operations 4.4.0, so unless you set customDirectives: { apolloUnmask: true } in your codegen config, the types should continue to get generated the same they do today.

For that @disableMasking directive, are you adding a document transform in codegen to handle that? I'm curious how you're achieving the unmasking here without that configuration in place.

[Cellule]

I did set customDirectives: { apolloUnmask: true } because I wanted to allow easy unmasking of fragments that are not intended to be masked.
The usual use case is to move data needed for a query into a fragment to get better typings and also use that fragment in mutation response to update the cache.
But that fragment is not intended to be used in a component so I wanted to unmask it.

For @disableMasking I have 2 things

• 1 plugin that gathers all fragment definitions and fragment spreads, then I validate that fragment spreads are using @unmask if the fragment definition has @disableMasking
• A document transform to remove @unmask before passing it to typescript-operations. I added an optional argument to keep the @unmask behavior in some cases (fragment of an interface for error response of mutations)

I also strip all @disableMasking, and other codegen-only directive I have, before calling typed-document-node plugin

[jerelmiller]

I think the thing I'm not quite understanding is why you'd want that data available runtime but not in types? I see you've made two statements here in terms of fragments:

I have some fragment on types that don't have clear ids, so I can't useFragment on it.

To make sure I'm understanding this case, is it safe to assume you're passing data as a prop to a child component here? And what you're asking for is to use @unmask because the data is non-normalized, but you want to make sure to enforce that only the child component is the only one that can access the fragment fields by ensuring the unmasked data in the parent is not available in that type. Does that sound right?

The usual use case is to move data needed for a query into a fragment to get better typings and also use that fragment in mutation response to update the cache.

I'm curious, where would you be consuming that data? Is there a reason you want to use @unmask here? Are there other components involved? Perhaps a small code example would be helpful. I'm not quite sure I'm following the structure here between the fragments and your components.

[Cellule]

To make sure I'm understanding this case, is it safe to assume you're passing data as a prop to a child component here? And what you're asking for is to use @unmask because the data is non-normalized, but you want to make sure to enforce that only the child component is the only one that can access the fragment fields by ensuring the unmasked data in the parent is not available in that type. Does that sound right?

Correct

Perhaps a small code example would be helpful. I'm not quite sure I'm following the structure here between the fragments and your components.

Yup! Give me a bit of time to get good examples in a readable format and I'll post it here

[jerelmiller]

Ok! On that first point, unfortunately this isn't a situation we really planned on since we wanted the runtime behavior to mimic the types as much as possible. I think your custom solution is probably the best bet here for now.

One other option I'll throw out there. Are you opposed to changing the fragment so that it operates on a normalized parent type? I don't know your app so I don't know how feasible this is, but this might also be a solution that avoids the need to do all the custom codegen stuff to make this work. This would also allow you to use useFragment in that component as well.

If we get a lot of demand for what you're asking for (masked parent type, but data available as a prop), this is something we could see if we could find a solution for natively. Ultimately though I think the right answer is to support non-normalized data in fragments via useFragment which is something we want to do eventually. We just couldn't figure out the right approach for this in 3.12 while providing the guarantees that useFragment does today (rerendering with cache updates). We felt it might add confusion if non-normalized data didn't rerender with cache updates.

[Cellule]

Answering your question first

One other option I'll throw out there. Are you opposed to changing the fragment so that it operates on a normalized parent type? I don't know your app so I don't know how feasible this is, but this might also be a solution that avoids the need to do all the custom codegen stuff to make this work. This would also allow you to use useFragment in that component as well.

I'm not opposed to it, but it would likely require a huge amount of refactoring and I'm not 100% it would be feasible in all cases.

Alright, I'll try to break down my use cases into different scenarios.
I'm using a fork of client-preset that acts like near-operation-file so typings, documents and various helpers are generated near the components.

Normal unmasking

// List.gql.ts
gql`
  fragment ListItem_Fragment on ListItem {
    id
    name
  }
`

gql`
  query ListQuery {
    list {
      ...ListItem_Fragment @unmask
    }
  }
`

gql`
  mutation EditListItemMutation($id: ID!, $name: String!) {
    editListItem(id: $id, name: $name) {
      item {
        ...ListItem_Fragment
      }
    }
  }
`

// List.tsx
import { useListQuery, useEditListItemMutation } from "./List.gql.gen"

function List() {
  const { data } = useListQuery()
  const [editListItem] = useEditListItemMutation()

  // Not a great example, but you get the point
  const handleEdit = (item: IListItem_Fragment) => {
    // ...
    editListItem({ variables: { id: item.id, name: item.name } })
  }

  // Here the component, from some third party library, expects an array of items, so there's no opportunity to use `useFragment`
  return <SomeListComponent items={data?.list} onEdit={handleEdit} />
}

Unormalized fragments

// TokenizedText.gql.ts
gql`
  fragment TokenizedText_Fragment on TokenizedText @disableMasking(maskTypings: true) {
    # No id available
    text
    tokens {
      type
      value
      ... on WorkOrderToken {
        workOrderId
      }
    }
  }
`

// TokenizedText.tsx
import { useTokenizedTextQuery } from "./TokenizedText.gql.gen"

// props.text expects a masked type of the fragment
function TokenizedText(props: { text: TokenizedText_Fragment }) {
  // Here we can't use `useFragment` because the fragment is not normalized
  const text = unmaskTokenizedText_Fragment(props.text)
  return <div>{text.text}</div>
}

// EntityX.gql.ts
gql`
  query EntityXQuery($id: ID!) {
    entityX(id: $id) {
      id
      description {
        ...TokenizedText_Fragment @unmask
      }
    }
  }
`

// EntityX.tsx
import { useEntityXQuery } from "./EntityX.gql.gen"
function EntityX(props: { id: string }) {
  const { data } = useEntityXQuery({ variables: { id: props.id } })
  // data.entityX.description should be masked in the typings so it cannot be used here, but the data is unmasked to be passed down to the TokenizedText component
  return (
    <div>
      <div>{data?.entityX.id}</div>
      <TokenizedText text={data?.entityX.description} />
    </div>
  )
}

Unnormalized fragments with maskTypings disabled

These are fragment used purely to simplify data fetching from an interface.
In this particular case, the fragment is used to fetch the response of a mutation and we want to unmask it to make it easy to inspect outside of a component.

// SingleEntityResponse.gql.ts
gql`
  fragment SingleEntityResponse_Fragment on ISingleEntityMutationResponse @disableMasking(maskTypings: false) {
    success
    errors {
      fieldPath
      fieldValue
      error
    }
  }
`

// List.gql.ts
gql`
  mutation EditListItemMutation($id: ID!, $name: String!) {
    editListItem(id: $id, name: $name) {
      ...SingleEntityResponse_Fragment @unmask
      item {
        ...ListItem_Fragment
      }
    }
  }
`

// List.tsx
import { useListQuery, useEditListItemMutation } from "./List.gql.gen"

function List() {
  const { data } = useListQuery()
  const [editListItem] = useEditListItemMutation()

  const handleEdit = async (item: IListItem_Fragment) => {
    // ...
    const response = await editListItem({ variables: { id: item.id, name: item.name } })
    // No need to unmask the response, it's already unmasked
    if (response.data?.editListItem.success) {
      // ...
    } else {
      // ...
    }
  }

  return <SomeListComponent items={data?.list} onEdit={handleEdit} />
}

Validation of @disableMasking to avoid common mistakes

The piece that I struggled with the most is how to properly document to devs which fragments are "safe" to use @nonreactive on.
Also, I want to slowly prepare for Apollo Data Masking, so if a fragment/component expects to receive data through props at runtime it needs to use @unmask while still keeping the type masked in most cases.
Technically, since we don't use Apollo Data Masking, I could not use @unmask on unnormalized fragments, but I'm opening ourselves up to major bugs when we'll try to switch to Apollo Data Masking.

gql`
  fragment SomeFragment on SomeType @disableMasking(maskTypings: true) {
    id
  }
`

gql`
  query SomeQuery {
    some {
      ...SomeFragment # Error missing @unmask
      ...SomeFragment @nonreactive # Error @nonreactive is not allowed on fragments that disabled masking
    }
  }
`

Conclusion

Right now I'm trying to find the best in between to respect the following criteria:

• Help transition to Fragment Masking as a whole while preparing for Apollo Data Masking
• Keep the developer experience as good as possible and avoid boilerplate and common mistakes
• Keep the generated code as clean as possible
• Make it easy to understand which fragments are safe to use @nonreactive on

I'm fine with keeping my custom solution, but I wanted to at least chime in about our use cases and if there's something we're doing terribly wrong or if there might be an option for official support either from Apollo or GraphQL Codegen I feel that now is a good time to open up the discussion

[jerelmiller]

Let me throw this out there. Our data masking docs just went live since we just published the release. There is some information in here about incremental adoption so maybe this answers a few questions about how we envision that path for existing apps: https://www.apollographql.com/docs/react/data/fragments#incremental-adoption-in-an-existing-application

Normal unmasking

Everything looks good here. Something I'll mention as well specifically with mutations is that the mutation callbacks that handle updating the cache (like the update callback) are unmasked so you have the full result type. Here is a good run down of which values are masked and which are unmasked in both mutations and subscriptions: https://www.apollographql.com/docs/react/data/fragments#working-with-other-operation-types

Just wanted to mention this so that you know that @unmask should be unnecessary with mutations unless you're specifically needing something from the resolved value on the promise that requires the full result set. We've tried to keep any API that does something with the cache as the unmasked value to keep cache writes simpler.

Unnormalized fragments

Ya unfortunately this is just not something we have support for in the way you're looking. Your custom solution still makes sense after seeing this. We didn't provide any TypeScript escape hatches to give you the best of both worlds while using @unmask.

Unnormalized fragments with maskTypings disabled

The case with a fragment for use with a mutation makes sense, though again check out the reference in case one of the callbacks makes sense here which gives you the full unmasked type/value without the need for @unmask (if it makes sense for your situation).

which fragments are "safe" to use @nonreactive on

I'd like to understand your definition of "safe" here, but if its related to that code sample, then I think you have that correct since I assume you'll want rerenders on cache updates.

If you can, I'd try and limit @disableMasking to only non-normalized cases and use useFragment for every case that you have normalized data. In the example with SomeFragment, I see id selected, so I'd recommend not using @unmask here anyways. Probably besides the point you're trying to convey here, but wanted to throw that out there.

As an aside, from a client perspective, it should be fine to use @nonreactive on masked fragments in case someone does make that mistake. In the case the fragment is masked, this just becomes a noop since data masking treats fragment spreads as @nonreactive anyways without the need to specify the directive.

Other than that, I don't know I have a better idea than what you're doing on how to enforce that you've added @disableMasking in the right places.

Help transition to Fragment Masking as a whole while preparing for Apollo Data Masking

Again, I'll point you to the incremental adoption docs because you might be able to leverage some of this as part of your upgrade path sooner than later. This would give you access to @unmask as well.

Unless you enable data masking in the client, the client should function the same as it does in 3.11.

---

While not ideal, I don't know that I have much better solutions than what you're doing right now without a native solution in place. Again I think we want to allow for non-normalized data with masking in the future, but until then I think this is your best bet.

[Cellule]

Everything looks good here. Something I'll mention as well specifically with mutations is that the mutation callbacks that handle updating the cache (like the update callback) are unmasked so you have the full result type. Here is a good run down of which values are masked and which are unmasked in both mutations and subscriptions: apollographql.com/docs/react/data/fragments#working-with-other-operation-types

Yeah in 90% of the case we don't even bother updating the cache manually for simple cases, we just let the magic of the cache work
As for the update method, right now I'm getting masked typings from GraphQL Codegen, maybe we need a way to somehow unmask everything in those methods?

Again, I'll point you to the incremental adoption docs because you might be able to leverage some of this as part of your upgrade path sooner than later. This would give you access to @unmask as well.

Yeah I'm considering it, but my biggest fear is dev education. Sure I can do a 1-time migration and it'll mostly work, but then I need to make sure devs either use proper fragment masking or use @unmask(mode: 'migration')
I think it'll be a baby step kind of thing of getting devs to start using fragment masking from codegen only and then look at Apollo's approach :)

Otherwise thanks for your input! I'll keep that in mind and indeed I hope that disableMasking will be a special case and not the norm.

Since we are prop drilling, right now we're using a failsafe where if useFragment fails to return data, we return the data passed down as props.
That mechanism would no longer work when enabling Data masking in Apollo however

One last thing that I haven't hit yet, but might become a problem is some devs "craft" data to look like the right data and pass that down in props. In such case the data wouldn't be in the cache and useFragment would fail.
We're working on fixing that though.

[Cellule]

Urgg I just saw the setIsSubmitting in my screenshot and I got so sad... please ignore that I'm fixing it to use const [revertAuditEntry, { loading: isSubmitting} ] = useRevertAuditEntryMutation() hahaha

[jerelmiller]

As for the update method, right now I'm getting masked typings from GraphQL Codegen, maybe we need a way to somehow unmask everything in those methods

You're on 3.12 correct? If so, thats odd because it should have the Unmasked helper applied to it:

apollo-client/src/core/types.ts

Line 196 in 36b93b9

result: Omit<FetchResult<Unmasked<TData>>, "context">,

🤔

[Cellule]

You're on 3.12 correct? If so, thats odd because it should have the Unmasked helper applied to it:

Oops 🙊 I hadn't updated yet haha
Looks good now

[jerelmiller]

Phew 🎉. For a second I thought 3.12.1 might have to be on its way 😆

[Cellule]

Alright then I'll close this issue for now, I don't think there's anything more to do on Apollo's side at this time.
I'll try to get our projects on board with Apollo Data masking as soon as I can and give feedback :)

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[jerelmiller]

That would be amazing! Hope all goes well when you do start using it! Appreciate the conversation!

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using our Community Forum or Stack Overflow.