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

Always retain __typename during data masking #12016

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

jerelmiller
Copy link
Member

While working on data masking for nested fragments, I stumbled on a potential pitfall with the existing solution; the data masking algorithm only retains exactly whats defined in the selection set. This means that unless __typename is defined in the fragment, this property was removed from the returned object. This is mostly ok if you're just working with the query and one level of fragments, but as you introduce nested fragments with data masking, this made it difficult to work with. If you forget to include __typename with data masking enabled, this means the from option passed to useFragment/watchFragment was invalid, so the value returned was empty. This could be solved in user-space by adding a __typename selection in the fragment, but we don't ask our users to do that with queries, nor does using watchFragment without masking behave this way today.

This PR retains __typename when present regardless of whether its present in the original document or not.

Copy link

changeset-bot bot commented Aug 21, 2024

⚠️ No Changeset found

Latest commit: 990b655

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

test("deep freezes the masked result if the original data is frozen", () => {
const query = gql`
query {
user {
__typename
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed all these __typenames from the existing tests because I want the test to more closely resemble what I expect to see from our users and how they will use this feature in their apps.

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most tested change ever. I love it!

@github-actions github-actions bot added the auto-cleanup 🤖 label Aug 22, 2024
@jerelmiller jerelmiller merged commit ddc0ecc into data-masking Aug 22, 2024
40 checks passed
@jerelmiller jerelmiller deleted the jerel/always-retain-typename branch August 22, 2024 14:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants