readQuery expects query variables to match the type stored in the cache, which isn't specified in the docs

[avinoamsn]

Intended outcome:

readQuery returns the expected value/object.

Actual outcome:

readQuery returns null, per #1542 & apollographql/apollo-feature-requests#1.

How to reproduce the issue:

When querying my application's database, objects IDs are returned as strings. However, the InMemoryCache cache stores these same IDs as numbers. Because the docs entry for readQuery doesn't specify strict comparison (or anything else), I thought that something like this would work fine:

const data = client.readQuery({
	query: ReadContactSystemRoles,
	variables: { id: contact.id }, // note that here, `contact.id` is a string, but its cache counterpart is a number
})

But this returns null every time. Instead, I have to convert to a number before passing the variable:

const data = client.readQuery({
	query: ReadContactSystemRoles,
	variables: { id: parseInt(contact.id) }, // `contact.id` is parsed to a number before being passed to the `variables` object
})

This produces the expected result.

Versions

System:
  OS: Linux 4.19 Debian GNU/Linux 10 (buster) 10 (buster)
Binaries:
  Node: 14.16.0 - ~/.nvm/versions/node/v14.16.0/bin/node
  Yarn: 1.22.15 - /usr/bin/yarn
  npm: 8.1.1 - ~/.nvm/versions/node/v14.16.0/bin/npm
npmPackages:
  @apollo/client: ^3.3.11 => 3.3.11 
  apollo-upload-client: ^16.0.0 => 16.0.0

[benjamn]

@avinoamsn It sounds like you have your answer: consistency of variable types is important for caching.

You seem to be suggesting, indirectly, that InMemoryCache should be insensitive to the distinction between string variables and number variables. I honestly do not think that's a behavior you would want in general, even though it happens to match your initial expectations in this one case. Any system that automatically performs type conversions is making decisions on your behalf that you might not have wanted or expected. This is why, for example, many JS developers use === by default and avoid ==.

If you care about type safety, I would recommend solving this problem at the TypeScript level, by giving your queries the type TypedDocumentNode<Data, Variables> (as generated by a tool like GraphQL Code Generator) rather than just DocumentNode. The Variables type parameter will be used to check the variables that you provide, which likely would have prevented any confusion here.

[avinoamsn]

@benjamn Apologies if my issue is unclear, but I'm not suggesting that any logic be changed within Apollo. I'm merely asking that the docs be updated to reflect this behavior (hence the title of this issue: ...isn't specified in the docs).

I would've submitted a PR myself, but I didn't see any Edit this page on GitHub (etc.) link on the page in question—but checking again, I now see the link in the sidebar, so I'll make an edit & close this issue accordingly.

I'll see about using TypedDocumentNode, since that seems like a pretty helpful addition to our pre-existing type system. Nevertheless, I don't appreciate the condescending tone you strike in presuming to answer a question that you—again—misinterpreted.