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

fix: unbound-method lint error by using arrow function syntax for fns declared on the ObservableQueryFields interface #11558

Merged

Conversation

alessbell
Copy link
Member

@alessbell alessbell commented Feb 1, 2024

Closes #11554.

Copy link

changeset-bot bot commented Feb 1, 2024

🦋 Changeset detected

Latest commit: 50b389d

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

github-actions bot commented Feb 1, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 38.14 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 45.95 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 43.49 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 33.88 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 31.8 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.23 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.21 KB (0%)
import { useQuery } from "dist/react/index.js" 5.2 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.27 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.5 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.58 KB (0%)
import { useMutation } from "dist/react/index.js" 3.51 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.73 KB (0%)
import { useSubscription } from "dist/react/index.js" 3.19 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.38 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.28 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 3.94 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.75 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.4 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 4.97 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.63 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.04 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 2.98 KB (0%)
import { useFragment } from "dist/react/index.js" 2.18 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.13 KB (0%)

Copy link

netlify bot commented Feb 1, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 50b389d
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/65c15120aefcc400087b766e
😎 Deploy Preview https://deploy-preview-11558--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.

@alessbell
Copy link
Member Author

Confirmed this would impact the docs. Any ideas on how we can make both happy, @phryneas? We could try @jerelmiller's idea of a "fake this" but that would also impact the type that ends up in the docs.

@@ -81,9 +81,11 @@ export interface ObservableQueryFields<
TVariables extends OperationVariables,
> {
/** {@inheritDoc @apollo/client!QueryResultDocumentation#startPolling:member} */
startPolling(pollInterval: number): void;
// startPolling(pollInterval: number): void;
startPolling: ObservableQuery<TData, TVariables>["startPolling"];
Copy link
Member

Choose a reason for hiding this comment

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

Alternative options:

Arrow function

Suggested change
startPolling: ObservableQuery<TData, TVariables>["startPolling"];
startPolling: (pollInterval: number) => void;

this: void,

Suggested change
startPolling: ObservableQuery<TData, TVariables>["startPolling"];
startPolling(this: void, pollInterval: number): void;

@alessbell alessbell marked this pull request as ready for review February 5, 2024 21:10
@alessbell alessbell requested a review from a team as a code owner February 5, 2024 21:10
@alessbell
Copy link
Member Author

Thanks @phryneas, this is ready for a final look!

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.

Looks great!

@@ -81,11 +81,13 @@ export interface ObservableQueryFields<
TVariables extends OperationVariables,
> {
/** {@inheritDoc @apollo/client!QueryResultDocumentation#startPolling:member} */
startPolling(pollInterval: number): void;
// startPolling(pollInterval: number): void;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// startPolling(pollInterval: number): void;

I think we could probably get rid of the commented old code 🙂. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, totally overlooked those :) Removed in 556b085

src/react/types/types.ts Outdated Show resolved Hide resolved
@github-actions github-actions bot added the auto-cleanup 🤖 label Feb 5, 2024
@alessbell alessbell changed the title fix: test fix for unbound method error which I expect to break the docs fix: unbound-method lint error by using arrow function syntax for fns declared on the ObservableQueryFields interface Feb 5, 2024
alessbell and others added 3 commits February 5, 2024 16:18
…ds' of github.com:apollographql/apollo-client into 11554-fix-unbound-method-error-on-observablequery-methods
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.

🚢 it

@alessbell alessbell merged commit 8cba16f into main Feb 5, 2024
29 checks passed
@github-actions github-actions bot mentioned this pull request Feb 5, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 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.

Apollo Client 3.9 causes linting error to be thrown when using refetch
3 participants