RFC: Data masking with fragments

[jerelmiller]

This work is part of the Data masking milestone. Follow the milestone for progress on this feature.

Background

When building complex applications today, Apollo Client users reach for useQuery to get data into their components. This is no surprise as this is considered a best practice by the Apollo Client documentation.

We are taking an initiative this year to change the recommendation to instead prefer fragment composition and fragment colocation as the standard pattern for building apps with Apollo Client. While the out-of-the-box experience with this pattern works, as recommended by our documentation, there are a few shortcomings of the existing solution:

• It’s easy to introduce implicit coupling on fragment data from query components¹. This makes the app more prone to breakage as child components are refactored and data requirements change.
• Cache writes rerender at the query component level, making fine-grained updates for fragment compoents difficult to do²

To alleviate these shortcomings, we are are introducing data masking into Apollo Client. Data masking is popularized by Relay and is useful to avoid implicit dependencies between components.

What is data masking?

Data masking is the functionality that provides access only to the fields that were requested in the component. This prevents implicit coupling between components by allowing components to access only the fields requested by that component. For query components, this includes all fields in a query not included in a GraphQL fragment. For fragment components, this includes the fields defined in the fragment definition.

Take the following as an example:

const query = gql`
  query {
    user {
      id
      name
      ...UserFields
    }
  }
`;

function App() {
  const { data } = useQuery(query)
  
  // loading state omitted for brevity
  
  return (
    <div>
      {data.user.name}
      <User user={data.user} />
    </div>
  )
}

function User({ user }) {
  // ...
}

User.fragment = {
  user: gql`
    fragment UserFields on User {
      age
      birthdate
    }
  `
}

In Apollo Client today, all user data is available to <App />. This means that <App /> can access fields such as age and birthdate, even though these fields were asked for by the fragment in Child. This creates an implicit coupling between <App /> and <User />. For example, if <App /> consumed user.age, and <User /> was refactored to remove age from the fragment, this would break the <App /> since age will no longer be loaded by the query.

Data masking solves this by only allowing the fields declared in that component to be accessible in the component that asked for that data. In this example, <App /> would only have access to user.id and user.name, but not user.age and user.birthdate since these were part of the fragment. <User /> would have access to user.age and user.birthdate, but not user.id or user.name since these fields are not part of the fragment.

The same applies to fragments that include nested fragments. One fragment cannot access data defined in a nested fragment. Take the following example:

const UserFragment = gql`
  fragment UserFragment on User {
    id
    name
    ...UserProfileFragment
  }
`

const UserProfileFragment = gql`
  fragment UserProfileFragment on User {
    age
    birthdate
  }
`

Here UserFragment can access id and name, but not age and birthdate. UserProfileFragment can access age and birthdate, but not id and name since these fields are not declared in the fragment.

Usage

With data masking enabled, you will issue queries the same as you do today. This is typically done with the useQuery or the useSuspenseQuery hook if you've adopted Suspense. The difference is that you will not be able to access fields defined in fragments from these components.

Instead, fields declared in fragments will be accessed through the useFragment hook*. To provide a nice developer experience, we plan update the from option in useFragment to support passing the entire parent object that contains the fragment as the value to this option. See the "Example" section below for a full code sample.

*Depending on technical feasibility and backwards compatibility, we may need to introduce a separate hook. This will be part of the exploratory work of this feature.

@defer

As part of this work, we will integrate useFragment with @defer and detect when the fragment is in-flight. This will integrate with React Suspense and cause the component to suspend until the fragment data has loaded.

Usage with non-suspenseful hooks

useFragment will not be limited to usage with suspenseful hooks however (such as useSuspenseQuery). Users may not yet be compatible with Suspense or may primarily be using useQuery to power their apps. Enabling suspense in these situations would be unwise and induce frustration.

To avoid this, we plan to make useFragment aware of the hook that produced the query to conditionally suspend the component. This means that Suspense will only be available when the query is produced by a suspenseful hook such as useSuspenseQuery.

Fetch policies with cached data

Users have the ability to leverage fetch policies to determine how to use cached data when consuming a query. For example, you can bypass the cache and force a network request with network-only, or read from the cache while fetching from the network with cache-and-network.

useFragment should be aware of this to mimic the query hook behavior when determining whether to return a cached result or suspend. For example, when a deferred fragment is used within a network-only query, the hook should suspend until the fragment is fulfilled, regardless of whether there is data in the cache. When used with a cache-and-network query, useFragment should provide the cached data and rerender when the network request finishes.

Non-React frameworks/libraries

The Apollo Client ecosystem is not limited to just React users. Libraries such as Apollo Angular and Vue Apollo provide view bindings for their respective view libraries. We plan to provide the foundation for these libraries to adopt this feature as well. This extends to users that use Apollo Client's core APIs.

We will be layering much of this work into Apollo's core query APIs, such as watchQuery and v3.10's watchFragment.

Render performance

Adopting data masking will include more benefits than just programming best practices. It will also provide performance benefits.

Today, cache writes to fields in a fragment definition cause the query component to re-render, regardless of whether that component consumes the data from the fragment or not. Depending on the depth of the component tree mounted beneath the query component, this may have significant performance implications. Many users avoid this by introducing additional query hooks in components further down the component tree. This however comes at the cost of additional network requests.

You can avoid the render performance implications today with the use of the @nonreactive directive combined with useFragment³. While this works well, it requires manual intervention.

With data masking enabled, this performance benefit will be an out-of-the-box feature. Because useFragment will be required to read data out of a fragment, we will target re-renders on the fragment components directly when there are cache writes to fields in the fragment.

Enabling this feature

Once this feature is introduced, it cannot be enabled automatically since this would constitute a breaking change. Instead, we will need to allow an opt-in to this feature. We plan to allow this in 2 ways:

Globally

We will allow data masking to be opted into globally by introducing a new option to ApolloClient. Enabling this option would automatically turn on data masking for every query that uses the client instance.

new ApolloClient({ 
  // This name is subject to change
  dataMasking: true
})

We will recommend that new users and applications opt into this feature immediately as the default. Smaller apps that have the capacity to migrate in an afternoon should also consider enabling this feature. We plan to make this the default in future major versions of Apollo Client with the eventual goal to deprecate this option and make this standard behavior.

Incrementally

We understand that large apps cannot stop everything and adopt this feature in its entirely. To allow large apps to migrate over time, we will allow this feature to be opted into incrementally.

We will do so by first requiring that users enable data masking globally, then allowing users to opt-out of data masking per query. While this approach seems counterintuitive to the goal of an incremental migration, this approach has some advantages:

• New queries are automatically masked. This takes out possible human error forgetting to enable data masking on any newly introduced query to the application.
• Over time, you are removing code to adopt data masking rather than adding code. This makes it easy to spot which queries in your application have not yet adopted the feature.

To make this approach feasible at scale, we will provide some out-of-the-box tools to handle the up-front work for you. For more information, see the “Migration tools” section below.

@unmasked

We plan to add support for a new client-only directive @unmasked that marks a query as unmasked. Any query marked as @unmasked will behave as it does today, allowing access to all fields, including those defined in fragments.

query MyQuery @unmasked {
  # ...
}

There is one distinction however. When using @unmasked, we will warn when accessing fields that would otherwise be masked if the directive were to be removed. This makes it easier to spot which fields will be affected by masking once the directive is removed from the query. When there are no more warnings, the directive should be safe to remove.

Migration tools

We will provide utilities that will ease migration in large apps to make it feasible to adopt this feature.

1. Codemod

We will provide a codemod that will crawl through the application and apply @unmasked fields to every query. This will handle queries used in gql tags and .graphql/.gql files.

1. ESLint plugin

We will provide an ESLint plugin with a rule that will warn for queries that contain @unmasked directives. This provides a more automated way to see areas of the codebase that have not yet adopted data masking. This also makes it possible to ban usage of the directive once data masking is fully adopted.

Example

const USER_QUERY = gql`
  query UserQuery {
    user {
      id
      name
      ...UserInfoFields
      ...UserAvatarFields
    }
  }
`

function App() {
  const { data } = useQuery(USER_QUERY);
  
  return (
    <div>
      {data.user.name}
      <UserInfo user={data.user} />
      <UserAvatar user={data.user} />
    </div>
  );
}
 

const USER_INFO_FRAGMENT = gql`
  fragment UserInfoFields on User {
     age
     birthdate
  }
`;

function UserInfo({ user }) {
  const { data } = useFragment({ 
    fragment: USER_INFO_FRAGMENT,
    from: user
  })

  return (
    <div>
      {data.age} - {data.birthdate}
    </div>
  ) 
}

const USER_AVATAR_FRAGMENT = gql`
  fragment UserAvatarFields on User {
    avatar {
      url
    }
  }
`;

function UserAvatar({ user }) {
  const { data } = useFragment({ 
    fragment: USER_AVATAR_FRAGMENT,
    from: user
  });

  return <img src={data.avatar.url} />
}

Open questions

Should we allow this for queries that use no-cache fetch policies?

useFragment is a cache API. It allows you to selectively read data out of the cache and re-render when that data changes. This gets tricky when used with no-cache queries. We'll either need add support to useFragment to allow it to be used without the cache, or we'll need to prevent usage with no-cache queries.

Depending on our final decision, this may warrant the introduction of a new hook to distinguish useFragment as a cache-only API.

Should this feature apply to cache.readQuery and cache.readFragment?

Cache APIs, such as cache.readQuery and cache.readFragment can be thought of as selectors for data in the cache. Data masking makes less sense here if you just need to read some arbitrary data out of the cache, especially since the queries/fragments you provide to these APIs do not actually have to be a query that was previously executed on the network. These APIs do not cause network requests and do not play a role in re-rendering your components.

Instead, we may prefer to build this into the client layer between the cache and the end usage. For example, using client.watchQuery and client.readQuery would be data-masked. You'd pair this with client.watchFragment and/or client.readFragment to masked fragment data from these APIs. The inherent risk with this approach is that it may cause confusion since the distinction between client.readQuery and client.cache.readQuery may not be apparent.

Can we allow fragment selections on non-normalized data?

Due to the way fragments work with the cache, useFragment is only able to read normalized data out of the cache. The from option creates the cache key used to look up the entity in the cache. With the planned update to the from option, should it be possible to read non-normalized data via useFragment?

The downside to allowing this is potential confusion on when this is allowed. Allowing non-normalized data to be selected when from originates from a query may make it feel like you can do this with any random fragment.

Will it be possible to pass the parent objects provided to from in React context, reactive vars, etc.?

It should theoretically be possible to pass around the parent objects that would normally be passed to child props in React context or other means of transporting values. I'm capturing this as an open question to make sure we are thinking about this while developing the feature.

How do cache writes work with data masking?

With data masking enabled, cache reads and writes are no longer symmetrical. We will need to explore ways to make cache.writeQuery make sense with this new paradigm.

Footnotes

1. Query components meaning components that initiate a GraphQL network request via a query hook, such as useQuery. ↩

2. This can be avoided with the combination of @nonreactive and useFragment today, but is more subject to error as it requires you to manually add @nonreactive in the appropriate places. ↩

3. For a more in-depth look at this feature, see @alessbell's blog post titled "Don’t Overreact! Introducing Apollo Client’s new @nonreactive directive and useFragment hook". ↩

[adamesque]

A few thoughts:

• Love the proposed change to from. If this is intended to allow fragment-reading functions to work with fragments defined on non-normalized types, that would be great to capture in the RFC (and I would love it)!!
• It isn't explicitly stated, but I presume that data masking would also apply to data returned from useFragment et. al when the passed fragment contains nested fragments. If that's the case, might also be good to call out, especially if that means fragments would also need to be tagged with @unmasked during the transition
• In our environment, graphql-codegen is widely used and has its own fragment masking features as part of its client-preset. It might be helpful to have a migration guide available.

[yaboi]

Are there any other ideas or suggestions on how to pass the parent object around beside React props? For example, if the component tree has a query at a top level but the colocated fragment isn't used for a series of nested children components? An example of this might be outside of the RFC here, but one of the things I like so much about useFragment right now is how it has removed the need for us to pass data around via React props.

[adamesque]

We'll either need add support to useFragment to allow it to be used without the cache, or we'll need to prevent usage with no-cache queries.

Would the proposed enhancement to from potentially help solve for this case?

Something we've wanted for a while is automatic query registration for fragments so that a loading flag (or suspense state) could be surfaced via useFragment when any query containing that fragment is in flight. If you were doing that kind of registration, you could potentially warn if an attempt is made to use useFragment with a no-cache query without passing the fragment data directly via the enhanced from arg.

[jerelmiller]

@adamesque thanks so much for the feedback!

If this is intended to allow fragment-reading functions to work with fragments defined on non-normalized types, that would be great to capture in the RFC

I would LOVE for this to be the case, though I'm still a bit technically uncertain if this is going to be possible.

I've added this as an open question to make sure this is captured. This is definitely going to be explored as a possibility.

I presume that data masking would also apply to data returned from useFragment et. al when the passed fragment contains nested fragments

YES! Thanks for calling this out! I've added this to the section on "What is data masking?" to make sure this is captured.

In our environment, graphql-codegen is widely used and has its own fragment masking features as part of its client-preset. It might be helpful to have a migration guide available.

Yes good call! Codegen compatibility is important, but capturing the differences between codegen's useFragment helper and Apollo's built-in data masking will be important as well. Thanks for calling this out to make sure we are thinking through this as we develop the feature.

Would the proposed enhancement to from potentially help solve for this case?

Thats the goal, though really what I get stuck on is the "confusion" aspect of this potentially. Our docs start with marketing useFragment as this:

The useFragment hook represents a lightweight live binding into the Apollo Client Cache

While its certainly possible that we change the "marketing strategy" on this hook, I just want to make sure that everything feels intuitive, otherwise we might end up with more frustration than solved problems.

I think this is just a matter of building it to work this way, then trying it out to see where the papercuts are. I plan to fully try out the new capabilities on our spotify showcase to make sure I can see it through the lens of the consumer. If all else fails, then we'll add a new hook to capture all of this functionality (though I'd prefer not to... naming is hard 🤣).

so that a loading flag (or suspense state) could be surfaced via useFragment when any query containing that fragment is in flight

This is planned as a followup to the data masking work 🙂. The data masking work should lay the ground work for us to determine when fragments are in flight so that we can layer in suspense functionality.

[jerelmiller]

@yaboi thats a great question! Any updates we do to useFragment will HAVE to be backwards compatible, otherwise its a breaking change. Everything you do today in terms of passing this around via context, reactive vars, etc. will continue to function as it does.

I really see the updates to from as more of a "when you use it this additional way, it adds these behaviors". BUT this is a good callout because it should theoretically be possible to put that parent object that you'd pass via props into React context or a reactive var and still get the same behavior. I'll add this as an open question though to make sure its captured in the RFC.

[blasterfiles]

Heyo, would love to see a small example of a unit test using the new from: option

Right now we're using cache.writeFragment + useFragment that just use an ID to introspect the cache.

In order to keep our tests atomic, its important we be able to mock the kind of object reference that goes into "from" easily

[jerelmiller]

@blasterfiles thats a great callout! I've added #11735 to track this. Thank you!

[stubailo]

Hi! This looks really cool. One question I had is, what will the TypeScript types look for user in this instance? How would one type "this prop is a thing I need to pass into useFragment", in a way that also ensures that the parent component passes in the right object?

[vladar]

We experimented a little bit with a custom @mask directive, which works similar to @skip at the cache reading level. This proved to be a pretty simple implementation (just a few line change in the readFromStore) and it showed a nice performance boost - both for the initial useQuery and subsequent updates (because unlike @nonreactive the result of useQuery isn't changing in optimism on fragment fields changes, so the cache doesn't even invoke watcher callback for parent useQuery).

With this directive it was also trivial to adjust GraphQL codegen to exclude fragment fields from generated types. So, it could be easier to migrate exiting apps with such "opt-in" approach rather than "opt-out" with @unmasked?

Funny enough, we still found @nonreactive valuable for some of our scenarios where we wanted to keep some fields in the result of the parent useQuery, while preventing them from triggering re-renders on data change.

[jerelmiller]

@stubailo to be honest this is still a bit of an unknown right now. We are absolutely aware that we need TS types to align nicely with the updates to useFragment here, but I cant't say we know exactly what that will look like quite yet. Stay tuned!

---

@vladar thats interesting to hear! Glad to hear you've had a good experience a masking-type solution! I'll be happy to have something first-class in the client as well here so that everyone can benefit 🙂.

So, it could be easier to migrate exiting apps with such "opt-in" approach rather than "opt-out" with @unmasked?

We've had discussions about the approach here and realize that an opt-out solution adds a bit of investment up-front, especially for large apps. I'll restate what I put in the RFC though on why we believe this is the right direction:

this approach has some advantages:

• New queries are automatically masked. This takes out possible human error forgetting to enable data masking on any newly introduced query to the application.
• Over time, you are removing code to adopt data masking rather than adding code. This makes it easy to spot which queries in your application have not yet adopted the feature.

We plan to make this process easier with migration tools so that its a more automated process. I strongly believe though the long-term benefits will outweight the up-front cost to enable it, even if your app remains in a state of "migration" for a long-period of time. This approach also prepares for a future where data-masking will become the default. This likely won't happen for another couple majors, but this is the long-term goal.

Funny enough, we still found @nonreactive valuable for some of our scenarios where we wanted to keep some fields in the result of the parent useQuery, while preventing them from triggering re-renders on data change.

That's very interesting! In my mind, data-masking will remove the need for @nonreactive in most cases, so its cool to hear you've still found uses for it!

[rbalicki2]

Absolutely love it!! Especially the thought that went into incremental adoption. I have a bunch of thoughts, mostly related to wording in this RFC, but some related to avoiding what I feel are mistakes that Relay made.

Overall, data masking is great and I think this is awesome, and I can't wait to see what y'all ship.

Clarity about when fragments suspend

• "detect when the fragment is in-flight". Fragments are not in flight. Maybe it would be more accurate to say e.g. "if the query from which the fragment was sourced is in flight while the fragment reads missing server data." And this makes clear that it's not that simple. What about:
  • queries that are re-executed?
  • queries that refetch part of the same tree?
  • What about mutations that have overlapping fields? etc.
  • fragments whose fields are not missing (i.e. they were previously cached), but whose fragment is deferred?
• The reason I bring this up is that you may encounter relationship changes leading to missing data. For example, a fragment selects best_friend { name }, and a follow up query selects best_friend { age } and incidentally discovers a new best friend. Now, you have a fragment with missing data.
  • In this situation, will random unrelated queries cause this fragment to suspend? This is problem that occurs in Relay.
• I think the connection to deferred fragments is incidental. A deferred fragment is a fragment for which there is an outstanding query and missing data. But that's not the only situation that that arises! Consider a query containing a fragment Foo on Query { node(id: $id) { __typename }}. The parent query is refetched with a new $id. The Foo fragment now has missing data and an outstanding query.

Whether a fragment suspends should be a fragment-level choice

• In relay, the fetch policy also controls whether fragments suspend. Or at least, that's true for the root query.
• Fetch policies conflate two things: whether to fetch and whether to suspend. e.g. network-only is fetch yes and suspend yes, while store-and-network is fetch yes, suspend no.
• It would be conceptually cleaner to separate these two.
• Anyway, suspense occurs at the fragment level (i.e. based on whether a fragment is reading missing data), so you should be able to control whether you want to suspend on a fragment-by-fragment basis.
  • A good default would be to inherit the suspense policy from the parent, but I haven't thought it through.
• In my case, being able to opt out of fragment-level suspense would make adoption of Relay 100x easier, as we are migrating from Redux to Relay, and our Redux setup doesn't support suspense. Adding suspense causes us to show fallbacks instead of "optimistically showing what data we can", and this causes regressions in our metrics.
• An added benefit of this might be in terms of type generation. If a fragment has @willAlwaysSuspendIfAnyDataIsMissing, you can generate types that are non-optional. But if a fragment has @willNeverSuspend, you can generate types that are optional. And then, enforce with the type system that you pass the appropriate suspensePolicy to useFragment.
• This is conceivably better for incremental adoption, as well.

Data masking and denormalized data

• At Pinterest, we are migrating from a Redux store (w/REST) to Relay/GraphQL. Our general plan is to implement Relay-compatible APIs that provide fragment data masking and can read from either the Relay store (i.e. they call the Relay APIs) or from denormalized data. This will allow us to incrementally adopt Relay APIs, and once a tree is fully migrated, turn on GraphQL and Relay.
• When reading from denormalized data, we "stash" the original data in a special symbol, i.e. if you read out { name, ...ChildFragment } you might receive { name: 'John', [secretSymbol]: DenormalizedData }
• So, data masking and denormalized data is possible. Happy to dive into more detail (e.g. about pagination, etc.) if you're interested

Prop drilling of fragment references

• Everyone also has a negative first impression if they see fragments with only fragment spreads. But this is fine! It's cheap, and preserves future flexibility.
• IMO your docs should heavily emphasize that this is a best practice, or people will fall into a pit of a failure where they pass props down multiple levels or store fragment references in context.

[jerelmiller]

@rbalicki2 first off, thank you so so much for this incredible feedback! There are definitely use cases that I hadn't quite thought of that will make excellent test cases. I'm going to take a bit of time and digest some of this and respond back with some of my thoughts when I've thought this through a bit more. I sincerely appreciate this!

[vladar]

@jerelmiller Thanks for your response, let me reply inline

We've had discussions about the approach here and realize that an opt-out solution adds a bit of investment up-front, especially for large apps.

For large apps it is indeed important to be able to experiment with the feature on a small subset of the codebase and once proven useful - gradually introduce it elsewhere. But I don't think opt-in vs. opt-out are mutually exclusive. In my mind the transition could be done in multiple stages:

1. Introducing an opt-in way in a minor version (e.g. @mask directive). This will significantly simplify adoption for big apps, as it is a non-breaking change, doesn't require major upgrade, and allows people to experiment with it and "feel" the benefits.
2. For the next major - switch to opt-out approach (but maybe still let people who decided to opt-out to have @mask supported for selected parts of the app)
3. For the subsequent major - make it the default with @unmask as the only way to opt-out

Such a gradual approach will make adoption much smoother and will also let you receive feedback earlier. Also, it doesn't conflict with the other stated benefits after major upgrade:

• New queries are automatically masked. This takes out possible human error forgetting to enable data masking on any newly introduced query to the application.
• Over time, you are removing code to adopt data masking rather than adding code. This makes it easy to spot which queries in your application have not yet adopted the feature.

Challenge with manual writes

Another challenge that would be great to cover in this RFC is manual writes to cache of operations containing masked fragments. This is a somewhat interesting topic, especially for strictly typed environments.

With data-masking reads and writes become non-symmetrical. Writes expect full data as input, whereas reads return partial data by default. This is a sort of ambiguity that could be resolved with different trade-offs.

(it is somewhat related to the section on manual cache.readQuery and cache.readFragment, but writes are even more tricky)

[jerelmiller]

@vladar appreciate the observation on cache writes! I've added this as an open question for now to ensure this is captured. I think this will take a bit of exploration and experimentation to get right.

In my mind the transition could be done in multiple stages

I think you and I are aligned in the goals here which is ultimately to make data masking the default behavior. Where your thinking differs from mine is the approach to get there. My thinking was that changing the default essentially means flipping the dataMasking flag from false -> true in a major version, then removing the flag altogether a major version after that. I worry that with your approach, its a bit of whiplash (i.e. first add @mask, then in the next major, swap all those for @unmask, though perhaps I'm misunderstanding your thinking here). The more friction we can avoid, the better. Doing the work up-front to globally enable and add @unmask everywhere is a one-time up-front cost, then from there its deleting code to adopt the feature (more on that below).

That being said, you make a good point here:

it is indeed important to be able to experiment with the feature on a small subset of the codebase and once proven useful - gradually introduce it elsewhere

Experimentation can be important, especially if the codebase spans multiple teams (common in large enterprise organizations). Many of these teams likely have processes in place where major changes like this need a vetting period where perhaps a team lead can create a proof-of-concept and pitch it to the rest of the org. I definitely don't want to ignore this.

I've entertained the idea of allowing a @mask directive as you mentioned, but I still have some reservations about this approach. Let me break down my points about this a bit further:

New queries are automatically masked.

This to me is a big one and where my goal is to try and ensure that once you've decided to adopt it, you're removing as much human error as possible. The @mask approach only works if you remember to add that directive for any new query added to your system. If you or someone on your team forgets this directive, this can add more work after-the-fact as it means having to revisit this query, add the directive, and test to ensure that unmasked fields aren't used in the wrong places. Hopefully this could be caught in a code review, but again, we're all humans and prone to make mistakes (especially if you are under a time crunch trying to deliver on a feature).

It can be incredibly helpful if Apollo Client can help drive adoption naturally without much thought or process on your end.

Over time, you are removing code to adopt data masking rather than adding code

Again to reiterate, if your team adopts the feature by using @mask rather than dataMasking: true + @unmask, you're 1) adding code to incrementally adopt this feature and 2) once you've fully adopted data masking across all your queries, you're having to go back through and remove the @mask directives in your existing queries. Furthermore, its difficult to tell at any given time whether or not you've actually fully adopted masking since it would require you to audit all your queries for a @mask directive. Any query without a @mask has the potential to break once you flip dataMasking to true.

This is what I mean by "removing code to adopt data masking". By default you're starting with @unmask, so to mask a query, you're removing the directive. Once you're removed all @unmask directives, there is no more work for you to do and you've fully adopted it across your codebase.

I find this approach easier to add linting rules to as well. During the transition period, you could set the linting rule to warn on @unmask usage. Once you've fully adopted masking (i.e. you've removed all @unmask directives), you switch that lint rule to error which helps prevent new queries from sneaking in @unmask.

---

At the end of the day, my goal here with this approach is to try and help teams avoid mistakes when adopting this feature. I'm willing to admit that I might be trying to push too hard toward the "pit of success" though.

I'll entertain the idea of adding a @mask directive or some other way to incrementally adopt in this manner. If we go that route, we'll likely recommend that you only use this for experimentation and prefer the @unmask approach with all the reasons I listed above. Again, I do think your point about experimenting on a subset of queries to get a feel for it is important and can be useful to prove out adoption. I don't want to dismiss this idea.

Appreciate the insight and conversation here!