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

Add failing test for changing variables with observableQuery #11581

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jerelmiller
Copy link
Member

Adding this as a point of discussion for our team on how core behavior should work relative to our React hooks.

This demonstrates a failing test for ObservableQuery core behavior where changing variables with returnPartialData: false will report partial data to next. This is in contrast to useQuery where we expect data in this same situation to return undefined:

it("should not return partial cache data when `returnPartialData` is false and new variables are passed in", async () => {
const cache = new InMemoryCache();
const client = new ApolloClient({
cache,
link: ApolloLink.empty(),
});
const query = gql`
query MyCar($id: ID) {
car(id: $id) {
id
make
}
}
`;
const partialQuery = gql`
query MyCar($id: ID) {
car(id: $id) {
id
make
model
}
}
`;
cache.writeQuery({
query,
variables: { id: 1 },
data: {
car: {
__typename: "Car",
id: 1,
make: "Ford",
model: "Pinto",
},
},
});
cache.writeQuery({
query: partialQuery,
variables: { id: 2 },
data: {
car: {
__typename: "Car",
id: 2,
make: "Ford",
model: "Pinto",
},
},
});
let setId: any;
const { result } = renderHook(
() => {
const [id, setId1] = React.useState(2);
setId = setId1;
return useQuery(partialQuery, {
variables: { id },
returnPartialData: false,
notifyOnNetworkStatusChange: true,
});
},
{
wrapper: ({ children }) => (
<ApolloProvider client={client}>{children}</ApolloProvider>
),
}
);
expect(result.current.loading).toBe(false);
expect(result.current.data).toEqual({
car: {
__typename: "Car",
id: 2,
make: "Ford",
model: "Pinto",
},
});
setTimeout(() => {
setId(1);
});
await waitFor(
() => {
expect(result.current.loading).toBe(true);
},
{ interval: 1 }
);
expect(result.current.data).toBe(undefined);
});
});

Copy link

changeset-bot bot commented Feb 9, 2024

⚠️ No Changeset found

Latest commit: 2dbaaf6

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

Copy link
Contributor

github-actions bot commented Feb 9, 2024

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 40.64 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 50.02 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 47.14 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 36.13 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 33.54 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.26 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.24 KB (0%)
import { useQuery } from "dist/react/index.js" 5.21 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.29 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 5.7 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.78 KB (0%)
import { useMutation } from "dist/react/index.js" 3.62 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.84 KB (0%)
import { useSubscription } from "dist/react/index.js" 4.42 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 3.48 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 5.51 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.17 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 5.01 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.66 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" 5.09 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production) 3.73 KB (0%)
import { useReadQuery } from "dist/react/index.js" 3.41 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 3.35 KB (0%)
import { useFragment } from "dist/react/index.js" 2.36 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.31 KB (0%)

Copy link

netlify bot commented Feb 9, 2024

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit 2dbaaf6
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/6751e92dd760b60008b2b13d
😎 Deploy Preview https://deploy-preview-11581--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.

@svc-apollo-docs
Copy link

svc-apollo-docs commented Dec 5, 2024

✅ Docs Preview Ready

No new or changed pages found.

@jerelmiller jerelmiller force-pushed the jerel/discuss-core-behavior branch from 32a3bac to 29343c1 Compare December 5, 2024 17:04
Copy link

pkg-pr-new bot commented Dec 5, 2024

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

commit: 2dbaaf6

@jerelmiller jerelmiller added this to the Release 4.0 milestone Dec 5, 2024
@jerelmiller
Copy link
Member Author

Moving this to 4.0 milestone so that we can loop it in with other work to ensure the core API is changed the same way everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants