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

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ryo-manba
Copy link

@ryo-manba ryo-manba commented Feb 1, 2025

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
Copy link

@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/

Copy link

netlify bot commented Feb 1, 2025

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

Visit the deploys page to approve it

Name Link
🔨 Latest commit e7ed70b

Copy link

changeset-bot bot commented Feb 1, 2025

🦋 Changeset detected

Latest commit: e7ed70b

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
Copy link

svc-apollo-docs commented Feb 1, 2025

❌ Docs preview failed

The preview failed to build.

Build ID: 4defa33138f50499acf7d309

Errors

react/api/react/components

  • Cannot find module '@microsoft/tsdoc/schemas/tsdoc.schema.json' from '/var/task/node_modules/.pnpm/@microsoft[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/@microsoft[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/@microsoft[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

@ryo-manba ryo-manba marked this pull request as ready for review February 1, 2025 03:36
@jerelmiller
Copy link
Member

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
Copy link
Author

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

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.

watchFragment reports result as complete when from object is not identifiable
4 participants