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

[Data masking] Add codemod to apply @unmask to all named fragments #12120

Merged
merged 24 commits into from
Nov 13, 2024

Conversation

jerelmiller
Copy link
Member

@jerelmiller jerelmiller commented Nov 12, 2024

Closes #11676

Adds a codemod that applies the @unmask directive to all named fragments in a file. This works for:

  • gql (or graphql) tagged template literals
  • graphql (or gql) functions
  • .graphql or .gql files

To run the codemod, use the following:

npx jscodeshift -t scripts/codemods/data-masking/unmask.ts \
  --extensions tsx \
  --parser tsx \
  path/to/file.tsx

By default this adds the @unmask directive with no arguments. To add it in migrate mode, use the --mode migrate option in the CLI:

npx jscodeshift -t scripts/codemods/data-masking/unmask.ts \
  --extensions tsx \
  --parser tsx \
  path/to/file.tsx \
  --mode migrate

You can customize the tag name with the --tag option, or use it to only apply to a specific tag name. This defaults to both gql and graphql tags when not provided.

npx jscodeshift -t scripts/codemods/data-masking/unmask.ts \
  --extensions tsx \
  --parser tsx \
  path/to/file.tsx \
  --tag gql

Pass --tag multiple times to parse more than one tag name.

@jerelmiller jerelmiller added the 🎭 data-masking Issues/PRs related to data masking label Nov 12, 2024
@jerelmiller jerelmiller added this to the Data masking milestone Nov 12, 2024
Copy link

changeset-bot bot commented Nov 12, 2024

🦋 Changeset detected

Latest commit: 65b06bb

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

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

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

@svc-apollo-docs
Copy link

svc-apollo-docs commented Nov 12, 2024

✅ Docs Preview Ready

No new or changed pages found.

Copy link

pkg-pr-new bot commented Nov 12, 2024

npm i https://pkg.pr.new/@apollo/client@12120

commit: 66dc5e8

Copy link
Contributor

github-actions bot commented Nov 12, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 40.6 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 50.99 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 47.57 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 36.31 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 33.56 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.26 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.25 KB (0%)
import { useQuery } from "dist/react/index.js" 5.22 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.3 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.71 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.78 KB (0%)
import { useMutation } from "dist/react/index.js" 3.63 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.85 KB (0%)
import { useSubscription } from "dist/react/index.js" 4.43 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 3.48 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.52 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.17 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 5.02 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.67 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.1 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.74 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.42 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.36 KB (0%)
import { useFragment } from "dist/react/index.js" 2.34 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.29 KB (0%)

@jerelmiller jerelmiller marked this pull request as ready for review November 12, 2024 07:24
Provide a codemod that applies `@unmask` to all named fragments for all operations and fragments. To use the codemod, run the following command:

```
npx jscodeshift -t node_modules/@apollo/client/scripts/codemods/data-masking/unmask.ts --extensions tsx path/to/app/
Copy link
Member Author

@jerelmiller jerelmiller Nov 12, 2024

Choose a reason for hiding this comment

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

I need to validate this is the correct path when the package is installed. So far I've been running this against the examples in this repo directly. If you'd like to do the same, the path to the unmask.ts script should be:

npx jscodeshift -t scripts/codemods/data-masking/unmask.ts --extensions tsx scripts/codemods/data-masking/examples/

Copy link
Member

Choose a reason for hiding this comment

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

image

Scripts are not copied to dist, so this never gets published to npm. :/

@phryneas
Copy link
Member

phryneas commented Nov 12, 2024

It works for .tsx and .graphql for me, but for the .ts file it crashes:

image

Removing the underlined lines it works... am I holding the tool wrong?

image

I was! It also needs a --parser=ts - adding that to the README

Comment on lines 7 to 9
```
npx jscodeshift -t node_modules/@apollo/client/scripts/codemods/data-masking/unmask.ts --extensions tsx path/to/app/
```
Copy link
Member

@phryneas phryneas Nov 12, 2024

Choose a reason for hiding this comment

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

Suggested change
```
npx jscodeshift -t node_modules/@apollo/client/scripts/codemods/data-masking/unmask.ts --extensions tsx path/to/app/
```
```
npx jscodeshift -t node_modules/@apollo/client/scripts/codemods/data-masking/unmask.ts --extensions tsx --parser=tsx path/to/app/
```

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.

Apart from that, works nicely!

It would be great if it could change existing @unmask to mode: "migrate", but that would be a bonus feature for a later time I guess ^^

Only thing we still have to figure out is distribution.

@github-actions github-actions bot added the auto-cleanup 🤖 label Nov 12, 2024
@jerelmiller
Copy link
Member Author

@phryneas

It would be great if it could change existing @unmask to mode: "migrate"

I considered that but left it for now because I thought that this might happen as a result of manually adding @unmask and figured its better to leave it alone than try and change it. Definitely one of those things though that have good arguments either way.

Should be easy to update though if we get feedback that it should update existing directives. If you feel strongly that we should have this out of the gate, I'd be happy to implement it.

@phryneas
Copy link
Member

Please don't let that be a blocker! It would be a nice to have and I'm not even sure if it's very useful, so let's wait on that for user feedback!

@jerelmiller
Copy link
Member Author

We still need to figure out the distribution strategy for the codemod, but that shouldn't hold us back from releasing the rc. Will merge now as we continue to explore that.

@jerelmiller jerelmiller merged commit 6a98e76 into release-3.12 Nov 13, 2024
32 of 36 checks passed
@jerelmiller jerelmiller deleted the jerel/unmask-codemod branch November 13, 2024 17:33
@github-actions github-actions bot mentioned this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-cleanup 🤖 🎭 data-masking Issues/PRs related to data masking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants