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

Delete mutations and their related queries #146

Closed
dustinfarris opened this issue Dec 7, 2016 · 13 comments
Closed

Delete mutations and their related queries #146

dustinfarris opened this issue Dec 7, 2016 · 13 comments

Comments

@dustinfarris
Copy link
Contributor

dustinfarris commented Dec 7, 2016

I have a scenario where user enters on a item index route

/items

The user clicks on the first item taking it to the detail route

/items/1

There is a delete button which the user clicks.

The delete action does a mutation

cashay.mutate('deleteItem', { variables: { itemId: 1 } });

and then redirects to the index route

/items

The item is still there.

My first thought, "this needs a mutation handler"

So I add one to the index route query:

const { data: { items } } = cashay.query(itemsQuery, {
  op: 'IndexRoute',
  mutationHandlers: {
    deleteProject(optimisticVariables, queryResponse, currentResponse) {
      debugger;  // What is going to happen here?  I'm not sure yet
    }
  }
}

The debugger never fires. Right before the handler is executed, I get this error:

Uncaught Error: Cache went stale & wasn't recreated. Did you forget to put a redux subscriber on IndexRoute?

I'm not sure how to interpret this.

@mattkrick
Copy link
Owner

mattkrick commented Dec 7, 2016

keep going! you'll need to build out the mutation handler.

maybe something like

if (optimisticVariables) {
  idx = currentResponse.findIndex(x => x.id === 1)
  currentResponse.splice(idx,1)
  return currentResponse
}

don't worry about immutability. Cashay manages its own dependencies so it'll normalize this, diff it, figure out what things changed, and then invalidate those components.

@dustinfarris
Copy link
Contributor Author

Thanks, but my debugger does not fire and it appears that the handler is never called.

The error occurs here: https://github.com/mattkrick/cashay/blob/master/src/Cashay.js#L594

Just a few lines before the first call to a mutation handler. Updating the mutation handler as you've suggested does not seem to be executed.

@mattkrick
Copy link
Owner

that's probably because you don't have a deleteItem mutation handler. it says deleteProject?

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Dec 8, 2016 via email

@dustinfarris
Copy link
Contributor Author

Here's screenshots

The action handler when the user clicks the delete button:

The index route stateToComputed

@dustinfarris
Copy link
Contributor Author

A little more info, here's a screenshot at the time just before the error is thrown

screenshot

Indeed, there is no cached query for IndexRoute, but there should be! This is after I've visited that page (and hence run that cashay query). So it should be there. Still investigating why it has disappeared.

@dustinfarris
Copy link
Contributor Author

dustinfarris commented Dec 9, 2016

Ok, I see what's happening now.

When I visit the index page, I fire off the "IndexRoute" query via cashay. When cashay gets the data back, it caches it in cachedQueries.IndexRoute.responses.

When I click on one of the projects, I am taken to the project detail page, which fires a new "ProjectDetail" query via cashay.

When the response for this second query comes back, cashay executes flushDependencies which removes the "IndexRoute" cached query.

Then I hit the delete button which triggers a mutation. The mutation sees that it has a mutation handler in the IndexRoute query and starts that process. Before executing the mutation handler, cashay checks for cached queries for that op, "IndexRoute", and throws an error when it doesn't find any.

@mattkrick I believe I have traced what is happening, but I'm not sure why it is happening. Can you offer some guidance?

@mattkrick
Copy link
Owner

When the response for this second query comes back, cashay executes flushDependencies which removes the "IndexRoute" cached query.

This is good! This is basically cashay's form of memory management. it aggressively invalidates responses that are no longer in the view layer. Since we aren't sure if the user will travel back to /item we don't create it right now, we lazily wait until they hit /item again.

Then I hit the delete button which triggers a mutation. The mutation sees that it has a mutation handler in the IndexRoute query and starts that process.

this is a really interesting use case (deleting yourself from your own leaf). that makes this super interesting!

so the easy answer:

this.transitionToRoute('index');
cashay.mutate(...)

I'm all about frameworks that don't let you write bad code. If cashay allowed you to mutate first, then it'd delete the data that is currently on the screen, which would leave you with an empty screen just before it redirected. Depending on how ember renders to the DOM, on slower machines you might see a flash of empty data before a redirect. By forcing you to redirect first, your user gets a cleaner experience with fewer rerenders and you get a more performant app. Granted, the error message sucks because there are many reasons that you might be seeing this.

If you have a good reason to mutate & then transition, I'd love to hear it! It wouldn't be difficult to write a fix for that use case (recreate the query if it is undefined) but that would cause a few unnecessary CPU cycles. Hope that all makes sense, & thanks for digging into it!

@dustinfarris
Copy link
Contributor Author

Thanks for explaining why this was happening.

My project is ember-cashay and I'm just writing a bunch of tests to prove out basic CRUD operations—which I completed tonight! To move things forward, I put the delete button next to each project on the index component instead of having each detail component have its own delete action. This made the problem go away for now, so I'm going to close this.

I do think there is a bug here though, and I'm fairly certain I'm going to run into this again at which point I'll open a new targeted issue.

For instance, I tried doing a transitionToRoute after an updateProject mutation (instead of delete), and while I didn't see the same error as I expected I would, the data in the index component was not refreshed. I added an updateProject mutation handler to it and it was never executed. Something is up with the way Ember and Cashay lose track of each other during a transition.

@dustinfarris
Copy link
Contributor Author

Reopening this:

Scenario A:

  • I have a query with a mutationHandler in a component on a certain route
  • and a mutation in a different component on a different route that affects the query
  • when i navigate away from the route with the query, cashay flushes it
  • the mutation attempts to resolve mutationHandlers and throws the error in the description of this issue

Scenario B:

  • I have a query without a mutationHandler in a component
  • and a mutation in a different component on a different route that affects the query
  • when i navigate away from the route with the query, cashay flushes it
  • mutation executes
  • navigating back to the original route sometimes shows the update, and sometimes does not.

I am seeing a lot of symptoms but having trouble triaging

@dustinfarris dustinfarris reopened this Jan 3, 2017
@dustinfarris
Copy link
Contributor Author

Without mutation handlers:

  • The mutation updates a record.
  • Hitting "back" to the index page does not show the change rendered
  • Change remains hidden until I access another, unrelated, item which triggers a server query
  • Hitting "back" to the index page now shows the original update rendered

This is weird to me because the query for the other item is unrelated to the original item that i mutated to start.

@dustinfarris
Copy link
Contributor Author

Going to reopen with a better description.

@dustinfarris
Copy link
Contributor Author

See #157 for a better triage.

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