Closed phryneas closed 2 months ago
Latest commit: e9d26e585714875176387a464cfe4138e3c2b980
The changes in this PR will be included in the next version bump.
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
Path | Size |
---|---|
dist/apollo-client.min.cjs | 38.94 KB (0%) |
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" | 47.66 KB (0%) |
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) | 45.2 KB (0%) |
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" | 34.23 KB (0%) |
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) | 32.09 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.2 KB (0%) |
import { useQuery } from "dist/react/index.js" (production) | 4.28 KB (0%) |
import { useLazyQuery } from "dist/react/index.js" | 5.68 KB (0%) |
import { useLazyQuery } from "dist/react/index.js" (production) | 4.75 KB (0%) |
import { useMutation } from "dist/react/index.js" | 3.59 KB (0%) |
import { useMutation } from "dist/react/index.js" (production) | 2.81 KB (0%) |
import { useSubscription } from "dist/react/index.js" | 3.63 KB (0%) |
import { useSubscription } from "dist/react/index.js" (production) | 2.78 KB (0%) |
import { useSuspenseQuery } from "dist/react/index.js" | 5.46 KB (0%) |
import { useSuspenseQuery } from "dist/react/index.js" (production) | 4.12 KB (0%) |
import { useBackgroundQuery } from "dist/react/index.js" | 4.99 KB (0%) |
import { useBackgroundQuery } from "dist/react/index.js" (production) | 3.64 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.35 KB (0%) |
import { useReadQuery } from "dist/react/index.js" (production) | 3.3 KB (0%) |
import { useFragment } from "dist/react/index.js" | 2.32 KB (0%) |
import { useFragment } from "dist/react/index.js" (production) | 2.27 KB (0%) |
Name | Link |
---|---|
Latest commit | e9d26e585714875176387a464cfe4138e3c2b980 |
Latest deploy log | https://app.netlify.com/sites/apollo-client-docs/deploys/668b9f78167bf6000846d246 |
Deploy Preview | https://deploy-preview-11806--apollo-client-docs.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Thinking out loud for a minute here. I actually kinda like the existing behavior purely for its explicitness. Take the following:
query GetTodo($id: ID = 1) {
# ...
}
and the following mocks:
const mocks = [
{
request: { query, variables: { id: 1 } }
},
{
request: { query }
}
]
Would you be aware that these are equivalent in the context of this query?
On the flipside, I totally understand the confusion on this. I guess it depends on what end of the spectrum you look at this. If you're thinking about it so that request.variables
matches the variables you pass to your query, then this change makes sense. If you're thinking about this from the perspective of what your GraphQL server would see, then the explicitness makes more sense.
Thoughts? At the very least, just wanted to start a discussion on this to see what we think.
Would you be aware that these are equivalent in the context of this query?
I think the important part here is the "in the context of this query" - in the context of this query, they could either be identical (this PR) or the second mock would be "impossible to reach" (current behaviour).
We could also think about warning about "impossible mocks", like
Warning: you specified a mock for a query
that can never be reached because the id
variable contains a default value and as a result can never be missing.
I'm not sure I like that too much, it seems like forcing an additional step on the user (and possible confusing them) for no good reason.
I'm not sure I like that too much
I would agree, I'm not super fond of it either.
Anyways, I think the pros outweigh the cons I mentioned, so I'm good with this one. Just wanted to have the conversation to make sure we at least discussed it. Thanks!
@phryneas is this something we can get in 3.11?
Good point - on it!
resolves #8023
This would resolve an old feature request and make it unnecessary to specify variables that match a mock query's default values.
Not sure if this counts as a patch or a minor though, and it will definitely not make it in 3.10.0, so I'm just opening it against
main
and we can discuss that separately.