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

Bug with type policies for local fields with arguments #349

Open
bennypowers opened this issue Sep 18, 2020 · 11 comments
Open

Bug with type policies for local fields with arguments #349

bennypowers opened this issue Sep 18, 2020 · 11 comments

Comments

@bennypowers
Copy link

bennypowers commented Sep 18, 2020

Steps to Reproduce

git clone https://github.com/bennypowers/cache-persist-client-args-repro.git
cd cache-persist-client-args-repro
npm ci
npm start

Repro repo:
https://github.com/bennypowers/cache-persist-client-args-repro
There's full code and more detailed explanations there in the README.

Description

Say you had a local field selected(page: $page) @client and a type policy which stored the value of selected based on whether and which page id was present.

query LaunchesQuery($page: String) {
  page @client @export(as: "page")
  launches(limit: 10) {
    id
    mission_name
    selected(page: $page) @client
  }
}
const cache = new InMemoryCache({
  typePolicies: {
    Query: { fields: { page() { return pageVar() ?? null; } } },
    Launch: {
      fields: {
        /**
         * Define `selected` on launch
         */
        selected: {
          keyArgs: ['page'],
          read(prev, { args, storage }) {
            /**
             * If there is no `page` arg, then the selected state should relate to the global list,
             * i.e, "is the launch selected on the root page"
             */
            if (!args?.page)
              return prev ?? true;
            /**
             * If there is a `page` arg, then the selected state should be scoped to that particular arg,
             * i.e. "is the launch selected on this specific page"
             */
            else {
              return storage[args.page] ?? false;
            }
          },
          /**
           * Store the selection state with regards to the current page
           */
          merge(_, next, { args, storage }) {
            storage[args.page ?? null] = next;
            return storage[args.page ?? null];
          },
        },
      },
    }
  }
}

If the cache is persisted, this code will run as expected on first page load (selected state will remain scoped to the page variable, if there is one). But users will see unexpected behaviour on subsequent loads, when the persisted cache is loaded up.

@bennypowers
Copy link
Author

Here's a live version of the reproduction:

https://happy-swirles-40f055.netlify.app/

@wtrocki
Copy link
Collaborator

wtrocki commented Sep 26, 2020

New version was released with completely different client.
There is example app that can help us to reproduce it but it seems that problem went away (at least after quick glance)

@bennypowers
Copy link
Author

bennypowers commented Sep 26, 2020

I've updated the reproduction app

https://happy-swirles-40f055.netlify.app/

I'm using apollo3-cache-persist, but unfortunately the bug remains the same.

(side note, why the frequent package name changes?)

@wtrocki
Copy link
Collaborator

wtrocki commented Sep 26, 2020

@bennypowers I do not have access to the original package when publishing and we still use apollo in some projects and often bump package dependencies so two packages need to exist. And original package cannot be used because original maintainer left and it is not responding to emails etc.

@bennypowers
Copy link
Author

I see.

If that's the case, I recommend contacting npm support. explain the situation with the original maintainer, link to this repo, and they should be able to give you control of the name on the npm registry

@wtrocki
Copy link
Collaborator

wtrocki commented Sep 29, 2020

@bennypowers I have been doing Open Source for last 10 years and this is the best reproduction project I have seen so far (seen many).

I have a feeling that this issue can be created inside Apollo's client. I end up debugging this but struggling to get a sense from it. I will try to take some time to understand the problem and see if we can get it done. We have landed new fix to master. Going to release it today.

@wtrocki
Copy link
Collaborator

wtrocki commented Sep 29, 2020

New version released.

@bennypowers
Copy link
Author

New version looks nice, but unfortunately did not fix the behaviour I'm noticing:

https://happy-swirles-40f055.netlify.app/

@wtrocki
Copy link
Collaborator

wtrocki commented Sep 30, 2020

Cache persist stores state only. I suspect that policy is not reexecuted on cache restore somehow. Need to isolate problem to confirm it, but that will explain it.

Can you tell me what is improper behaviour.
Is store containing wrong values?
Is UI containing wrong values despite cache being ok?

@bennypowers
Copy link
Author

This screenshot it taken directly following the reload step listed in the repro. The console shows the current state of the persisted cache's data immediately after reload. Launch:1 should display as selected.

Screen Shot 2020-09-30 at 10 24 08

@bennypowers
Copy link
Author

Oh, and check this out:
Screen Shot 2020-09-30 at 12 28 49

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

No branches or pull requests

2 participants