apollo-cache-persist icon indicating copy to clipboard operation
apollo-cache-persist copied to clipboard

Bug with type policies for local fields with arguments

Open bennypowers opened this issue 5 years ago • 11 comments

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 avatar Sep 18 '20 10:09 bennypowers

Here's a live version of the reproduction:

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

bennypowers avatar Sep 18 '20 10:09 bennypowers

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)

wtrocki avatar Sep 26 '20 14:09 wtrocki

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?)

bennypowers avatar Sep 26 '20 17:09 bennypowers

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

wtrocki avatar Sep 26 '20 17:09 wtrocki

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

bennypowers avatar Sep 26 '20 17:09 bennypowers

@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 avatar Sep 29 '20 18:09 wtrocki

New version released.

wtrocki avatar Sep 29 '20 19:09 wtrocki

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

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

bennypowers avatar Sep 29 '20 19:09 bennypowers

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?

wtrocki avatar Sep 30 '20 05:09 wtrocki

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 avatar Sep 30 '20 07:09 bennypowers

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

bennypowers avatar Sep 30 '20 09:09 bennypowers