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

useQuery: adjust for rules of React linting #11853

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented May 17, 2024

Another one for #11511...

Copy link

changeset-bot bot commented May 17, 2024

🦋 Changeset detected

Latest commit: ae0cb47

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

Copy link
Contributor

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 38.66 KB (+0.03% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 47.43 KB (+0.04% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 45 KB (+0.04% 🔺)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 34.2 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 32.05 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.23 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.22 KB (0%)
import { useQuery } from "dist/react/index.js" 5.3 KB (+0.36% 🔺)
import { useQuery } from "dist/react/index.js" (production) 4.38 KB (+0.36% 🔺)
import { useLazyQuery } from "dist/react/index.js" 5.54 KB (+0.27% 🔺)
import { useLazyQuery } from "dist/react/index.js" (production) 4.61 KB (+0.26% 🔺)
import { useMutation } from "dist/react/index.js" 3.52 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.74 KB (0%)
import { useSubscription } from "dist/react/index.js" 3.21 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.4 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.44 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.1 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.96 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.61 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.07 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.72 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.33 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.27 KB (0%)
import { useFragment } from "dist/react/index.js" 2.29 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.23 KB (0%)

@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 39573,
"dist/apollo-client.min.cjs": 39607,
Copy link
Member Author

@phryneas phryneas May 17, 2024

Choose a reason for hiding this comment

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

This PR actually doesn't change any behaviour, it just tries to give the lint rule a fighting chance of linting this hook. I'm not happy that we get this kind of size increase for that.

We definitely need a rewrite of this, soon!

Comment on lines +554 to +555
// React Hook "React.useMemo" cannot be called in a class component.
// eslint-disable-next-line react-hooks/rules-of-hooks
Copy link
Member Author

Choose a reason for hiding this comment

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

Ignore the linter error here for now - it goes away once we actually pull that rule in.

Copy link

netlify bot commented May 17, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit ae0cb47
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/66475882864e1a0008c3dbe2
😎 Deploy Preview https://deploy-preview-11853--apollo-client-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

// there is nothing to really do here with `disableNetworkFetches`,
// we need to reference it here to ensure the effect depends on it
// and resubscribes when it changes and the component is re-rendered by chance
disableNetworkFetches;
Copy link
Member

Choose a reason for hiding this comment

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

What an odd one! Would love a rewrite of this hook to try and remove this oddity 😆

Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

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

Creative way to get this linted. Thanks!

@github-actions github-actions bot added the auto-cleanup 🤖 label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants