Fix `watchFragment` reports `complete=false` when from object is not identifiable

[ryo-manba]

Closes #12003

Fixes an issue where watchFragment incorrectly reported complete: true when the from object was not identifiable (cache.identify returned undefined).

This was created from the branch adds the failing test. It might be better to merge that one first.

[apollo-cla]

@ryo-manba: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[netlify[bot]]

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	6dc4c34487d2eedc7961425ee32a5e9de6cef78f

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 739b62f50914a1b90b729102a71296ece3f5ab8a

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

[svc-apollo-docs]

❌ Docs preview failed

The preview failed to build.

Build ID: ba84bc7fd4e44a0393adbc41

Errors

react/api/react/components

• Cannot find module '@microsoft/tsdoc/schemas/tsdoc.schema.json' from '/var/task/node_modules/.pnpm/@[email protected]/node_modules/@microsoft/tsdoc-config/lib' react/api/react/hooks

• Cannot find module '@microsoft/tsdoc/schemas/tsdoc.schema.json' from '/var/task/node_modules/.pnpm/@[email protected]/node_modules/@microsoft/tsdoc-config/lib' react/data/queries

• Cannot find module '@microsoft/tsdoc/schemas/tsdoc.schema.json' from '/var/task/node_modules/.pnpm/@[email protected]/node_modules/@microsoft/tsdoc-config/lib' react/errors

• Transformer not found for component "DisplayClientError" react/local-state/managing-state-with-field-policies

• Could not parse expression with acorn

[jerelmiller]

Hey @ryo-manba 👋

Thanks for your patience!

I'd actually like to dig a layer deeper and see if we can fix the issue at the root. It looks like anything having to do with a Cache.DiffResult is affected by this issue. Since watchFragment calls cache.watch() under the hood, the issue is somewhere inside the result returned from that.

I can also reproduce the incorrect result using cache.diff with the following test:

test.only("reports diffs incorrectly", async () => {
  const cache = new InMemoryCache();

  console.log(
    cache.diff({
      query: cache["getFragmentDoc"](gql`
        fragment FooFragment on Foo {
          foo
        }
      `),
      id: cache.identify({}),
      returnPartialData: true,
      optimistic: true,
    })
  );

  await new Promise((res) => {
    const w = cache.watch({
      query: cache["getFragmentDoc"](gql`
        fragment FooFragment on Foo {
          foo
        }
      `),
      id: cache.identify({}),
      returnPartialData: true,
      immediate: true,
      optimistic: true,
      callback: (diff) => {
        console.log("callback", diff);

        res(void 0);
      },
    });
  });
});

If you paste and run this, you'll notice that cache.diff also returns a complete: true, even though it has no data.

From what I can tell, that getFragmentDoc thing is what triggers the issue. If I replace the document with a plain query, the data is properly reported as complete: false.

Let's see if we can fix it in the cache itself. Is this something you'd like to look into further?

[ryo-manba]

Thank you for the information! I'll dig into it a bit more.

[ryo-manba]

@jerelmiller Sorry for the delay! Implementation is complete and ready for review. Thanks for your patience!

[jerelmiller]

Hey @ryo-manba 👋

Thanks a bunch! Let me put this up for discussion on our next team meeting. We are very close to releasing versions 3.14 and 4.0 very soon so we'll need to figure out where this change fits best. Thanks again for following through!

[jerelmiller]

Hey @ryo-manba! Thanks for your patience!

We'd like to get this fix in 4.0, though after looking at this, we'd like to fix the cache.diff method so that it can work with fragments directly without having to create a query document out of it. That would make it much easier to detect whether we need an id or can use ROOT_QUERY as a fallback for actual queries.

I'll push some changes on top of this PR in the coming days to get that fix in place (I'm currently focused on writing some documentation for 4.0 before I can get to this).