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

Cache went stale and wasn't recreated #157

Open
dustinfarris opened this issue Jan 5, 2017 · 7 comments
Open

Cache went stale and wasn't recreated #157

dustinfarris opened this issue Jan 5, 2017 · 7 comments

Comments

@dustinfarris
Copy link
Contributor

dustinfarris commented Jan 5, 2017

I'm not sure where this is breaking down either in Cashay or in my own code; but here's the reproducible scenario:

Note: I typed this up by hand (no copy+paste) so if there's some stupid syntax error its probably just a typo in this GH issue alone.

The SETUP

Two routes:

  • /maps
  • /maps/:map_id

Starting with the second route: a detail page that can create and update maps. Creating is done by clicking "NEW" on the index page, which triggers an action that generates a UUID, sets a isCreatingNewMap flag in redux state, and redirects to the detail page.

The detail page has two queries and two mutations.

The two mutations are createMap and updateMap

The queries are identical except one has @cached and is used when the map is being created. This is so the createMap mutation knows to grab subfields.

This is the relevant piece of the MapDetail component:

/**
 * map detail component
 */

const cachedMapQuery = `
{
  map @cached(id: $map_id, type: "Map") {
    id
    name
  }
}
`;


const mapQuery = `
{
  map (id: $map_id) {
    id
    name
  }
}
`;


const mutationHandlers = {
  createMap(optimistic, response, current) {
    if (optimistic) {
      Object.assign(current.map, optimistic.new_map.map_details);
      return current;
    }
  },
  updateMap(optimistic, response, current) {
    if (optimistic) {
      Object.assign(current.map, optimistic.map_details);
      return current;
    }
  }
};


const stateToComputed = (state, attrs) => {  // `attrs` is like `props` in React
  
  const query = state.isCreatingNewMap ? cachedMapQuery : mapQuery;
  
  const { data: { map } } = cashay.query(query, {
    op: state.isCreatingNewMap ? 'map-detail-cached' : 'map-detail',
    key: attrs.map_id,
    variables: { map_id: attrs.map_id },
    mutationHandlers
  });
  
  return { map, isCreatingNewMap: state.isCreatingNewMap };
};


const MapDetail = Component.extend({

  isNewMap: oneWay('isCreatingNewMap'),  // this allows me get/set without modifying the parent
  
  saveMap: task(function *(changeset) {
    const map_id = this.get('map_id');  // from `attrs`
    let ops;
    if (this.get('isCreatingNewMap')) {
      ops = {
        'map-detail-cached': map_id,
        'maps-index': true,
      };
    } else {
      ops = {
        'map-detail': map_id,
        'maps-index': true,
      };
    }

    let promise;

    if (this.get('isNewMap')) {
      this.set('isNewMap', false);
      promise = cashay.mutate('createMap', {
        ops,
        variables: {
          new_map: {
            id: map_id,
            map_details: changeset.get('change')
          }
        }
      });
    } else {
      promise = cashay.mutate('updateMap', {
        ops,
        variables: {
          id: map_id,
          map_details: changeset.get('change')
        }
      });
    }
    
    yield promise.then(response => {
      if (response.error) {
        this.set('status', 'error');
      } else {
        this.set('status', 'saved');
      }
    });
  })
});

Back to the index page, this is the relevant piece of the MapsIndex component:

/**
 * maps index component
 */

const mapsQuery = `
{
  maps {
    id
    name
  }
}
`;


const mutationHandlers = {
  createMap(optimistic, response, current) {
    if (optimistic) {
      current.maps.push({
        id: optimistic.new_map.id,
        ...optimistic.new_map.map_details,
      });
      return current;
    }
  },
  updateMap(optimistic, response, current) {
    if (optimistic) {
      // I can't just Object.assign here because Ember observes everything
      // and gets cranky if you mutate without using its special helpers.. this 
      // behavior is going away eventually—I hope.
      const map = current.maps.findBy('id', optimistic.id);
      const index = current.maps.indexOf(map);
      current.maps.splice(index, 1, {
        ...map,
        ...optimistic.map_details
      });
      return current;
    }
  }
};


const stateToComputed = () => {
  
  const { data: { maps } } = cashay.query(mapsQuery, {
    op: 'maps-index',
    mutationHandlers
  });

  return { maps };
};


const MapsIndex = Component.extend({
});

The SCENARIO

Part 1 (The happy version)

Step 1 - User visits index

Cashay fires a server query:

{
  maps {
    id
    name
  }
}

All maps are displayed with their name

Step 2 - User clicks "NEW"

And is taken to a detail page with a freshly minted UUID in the URL

No server request. Cashay executes cachedMapQuery shown in the component code above.

Step 3 - User fills in name, triggers Save

Cashay executes a mutation:

map-detail optimistic mutation handler fires!
maps-index optimistic mutation handler fires!

then sends a server request

mutation ($new_map: NewMapInput!) {
  createMap(new_map: $new_map) {
    name
    id
  }
}

Step 4 - User navigates back to index

As part of this transition away, the detail route dispatches to turn off isCreatingNewMap. This triggers a redux subscribe callback. The MapDetail component, which has not been torn down yet, runs stateToComputed one more time which triggers a cashay.query, but this time using mapQuery instead of cachedMapQuery (shown in the component code above). This triggers a (unnecessary?) server request:

query ($map_id: ID!) {
  map(id: $map_id) {
    id
    name
  }
}

The transition completes and the user is now on the index page. The new map appears under the rest with its name. Everyone is happy so far.

Step 5 - User clicks the new map going back to its detail page

As in step 4, the map is looked up using mapQuery (the one without @cached). No new server queries—presumably because of the side-effect query in step 4, Cashay must be using the cache from that query.

Step 6 - User edits the name of the map and triggers save

map-detail optimistic mutation handler fires!
maps-index optimistic mutation handler fires!

And Cashay kicks off a mutation request to the server:

mutation ($id: ID!, $map_details: MapDetailsInput!) {
  updateMap(id: $id, map_details: $map_details) {
    name
    id
  }
}

Step 7 - User navigates back to the index page

No new server queries. The maps new name appears as it should.


Part 2 (The sad version)

The difference here is we'll be editing an existing map's name, instead of creating a new map and then editing it.

Step 1 - User visits index page

Cashay fires a server query:

{
  maps {
    id
    name
  }
}

All maps are displayed with their name

Step 2 - User clicks an existing map and taken to detail page

Cashay executes mapQuery in the component as shown in the component code above:

query ($map_id: ID!) {
  map(id: $map_id) {
    id
    name
  }
}

Step 3 - User edits the map's name and triggers Save

map-detail optimistic mutation handler fires!
maps-index optimistic mutation handler throws error!

@dustinfarris dustinfarris changed the title HELP! Cache went stale and was not re-created! Cache went stale and wasn't recreated Jan 6, 2017
@dustinfarris
Copy link
Contributor Author

The error happens when I leave the index page and on the new page cashay queries new data from the server.

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Jan 7, 2017

Ok, starting to understand:

  • when a server response comes back, cashay flushes "dependencies"
  • this is why i only see this problem when loading an unseen map detail component
  • cashay keeps the maps-index response until an unseen map detail component triggers a new server query, at which point cashay flushes the maps-index response
  • executing a mutation then throws the error when it tries to trigger the mutationHandler for maps-index

What I'm still struggling with is how, if the maps-index response is flushed, does the maps-index component continue to render. For example if I:

  • navigate to index triggering maps-index query (server query triggered)
  • navigate to map detail triggering map-detail query (server query triggered)
    • maps-index response is flushed at this time
  • navigate back to index (no new server query)
    • maps-index component renders all the maps! where is this data coming from if the response is flushed?

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Jan 8, 2017

Getting closer (without mutation handlers this time):

When the user visits maps index, cashay stores the result and entities in redux, e.g.:

{
  cashay: {
    entities: {
      Map: {
        1: { id: "1", name: "First Map" }
        2: { id: "2", name: "Second Map" }
        3: { id: "3", name: "Third Map" }
      }
    }
    ops: {
      maps-index: { variables: {}, status: "complete" .. }
    }
    result: {
      maps: [
        "Map::1"
        "Map::2"
        "Map::3"
      ]
    }
  }
}

cashay also caches a "response" object in memory, which, as far as i can tell at this point, is the denormalized result of the query, be it from the server, or from redux (or both i suppose in some cases).

So long as this "response" object exists, cashay will not ask redux, much less the server for an update. This is why mutationHandlers are necessary when there is a "response" object" in memory for a particular query.


When my user clicks a specific Map, they are taken to a detail page that executes a new "map-detail" query. As part of this operation, cashay flushes the above-mentioned response object for the maps-index operation. Then let's say the user executes an update mutation to change the name of the map. Cashay updates redux so it looks something like:

    entities: {
      Map: {
        1: { id: "1", name: "CHANGED NAME!" }
        ...

When the user navigates back to the maps index page, cashay flushes the response for the map-detail query. Cashay also sees that it no longer has a response object for the maps-index query. So it presumably checks redux for an existing result. And it finds one! So it pieces together a new response, but entirely from local state (redux). Because cashay updated entities during the mutation, the new response for the maps-index query contains the updated name. All is good.


Here's where it breaks down

Then the user clicks the same map and is taken back to the detail page. Cashay sees no "response" object for map-detail (it was flushed), but is able to piece together a new response from redux. Because no server query is required, the maps-index response object is not flushed!

The user changes the name again. Cashay executes a mutation.

The user navigates back to the index and doesn't see the change!

This is because cashay found that the response object for maps-index was still there and used that instead of checking redux.

How do we force flush a response when the user leaves the maps-index page but has not triggered a new server query?


Another way it breaks down

Then the user access yet another map which again flushes the response for the maps-index query.

Then, the user clicks a big 'ol PLUS icon in the navbar to add a new map.

Then the user fills out a new map name and mutation is executed. There are now 4 maps on the server.

The user navigates back to the maps index page, but sees just 3 maps!

This is because cashay, again, saw that there was no longer a denormalized response for the maps query, but there is still a result entry for the query. But that value (still) looks like this:

    result: {
      maps: [
        "Map::1"
        "Map::2"
        "Map::3"
      ]

Still 3 maps.

How do we update the result for "maps"? Can it be done without re-triggering a server request when we already have all the data we need?


@mattkrick i think this issue is ready for your eyes. This last comment I think summarizes the problem pretty well, but see above for more context.

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Jan 8, 2017

Side note: ☝️ I suspect that this problem goes away when @live is introduced. But I'm not quite there yet with my server (Elixir/Phoenix) so I'm stuck with the stale result data shown above for now.

@mattkrick
Copy link
Owner

there's a lot to dig into & i'm not sure i grok it all, but if stale data is getting to the view layer, then it has more to do with the ember integration i think. you shouldn't ever have to manually flush a result because the result will change whenever a mutation occurs as long as there is a mutation handler.

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Jan 11, 2017 via email

@dustinfarris
Copy link
Contributor Author

@mattkrick there may very well be some nuance at play with the Ember view layer, but I think there is a real bug here, and I think the reason you may not have run across it as much or at all in the action project is because your use of @live circumvents your need for mutationhandlers in this kind of scenario.

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