MockLink: add query default variables if not specified in mock request

[phryneas]

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.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7249ff6

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

[github-actions]

size-limit report 📦

Path	Size
dist/apollo-client.min.cjs	38.62 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs"	46.5 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production)	44.05 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js"	34.17 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production)	32.06 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.28 KB (0%)
import { useQuery } from "dist/react/index.js" (production)	4.37 KB (0%)
import { useLazyQuery } from "dist/react/index.js"	5.52 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production)	4.59 KB (0%)
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.46 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production)	4.12 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js"	4.94 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production)	3.59 KB (0%)
import { useLoadableQuery } from "dist/react/index.js"	5.05 KB (0%)
import { useLoadableQuery } from "dist/react/index.js" (production)	3.71 KB (0%)
import { useReadQuery } from "dist/react/index.js"	3.31 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production)	3.25 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%)

[netlify]

✅ Deploy Preview for apollo-client-docs ready!

Name	Link
🔨 Latest commit	7249ff6
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-client-docs/deploys/66335ee9501c8c000873c577
😎 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.

[jerelmiller]

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.

[phryneas]

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.

[jerelmiller]

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!