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

Improving immediate mutate + later revalidation #157

Closed
Daiz opened this issue Nov 27, 2019 · 9 comments · Fixed by #1745
Closed

Improving immediate mutate + later revalidation #157

Daiz opened this issue Nov 27, 2019 · 9 comments · Fixed by #1745
Labels
feature request New feature or request good first issue Good for newcomers

Comments

@Daiz
Copy link

Daiz commented Nov 27, 2019

The documentation for mutate talks about local mutation for faster feedback, but the documented way to use it only mutates after a promise has resolved - in other words, not immediately.

As a result, I've found myself writing code in this kind of pattern:

mutate(path, { ...data, patch }, false); // mutate immediately, don't revalidate
await patchData(patch); // await the actual API call
trigger(path); // trigger revalidation afterwards

Technically, if patchData also returns the full new data, you could also do it like this:

mutate(path, { ...data, patch }, false); // mutate immediately locally, don't revalidate
mutate(path, patchData(patch), false); // update with API-backed result from patchData

Which is actually also present in the documentation. But it is kinda janky to need two subsequent mutate calls for this...

One backwards-compatible way to deal with this could be to extend mutate to accept options as the third parameter, and introduce initialData much like present in useSWR. Then you could do something like this:

// mutate immediately with `initialData`,
// then replace with result from patchData when it resolves
mutate(path, patchData(patch), { initialData: { ...data, patch }); 

In case the patchData function (or equivalent) doesn't return the full data, the third argument could also be augmented to accept a Promise, eg.

// wait until the Promise from patchData resolves before triggering revalidate
mutate(path, { ...data, patch }, patchData(patch))

I feel both these changes would make using mutate a lot more ergonomic overall.

In addition, I haven't checked the code to see how easy this would be to implemented, but another nice possibility would be to keep the local data (ie. non-promise second argument if third arg is a promise, anything in initialData) provided to mutate in some sort of "unverified cache", such that in case the provided promises throw, SWR could automatically roll the data back to "last known good value" (ie. the last real API response for that path).

@pacocoursey pacocoursey added the feature request New feature or request label Nov 28, 2019
@johot
Copy link

johot commented May 30, 2020

I also find this very odd. Like you say the whole idea seems to be to mutate directly, then revalidate at a later time to make sure everything wen't like it should.

If I have a slow API and use mutate like in the documentation samples, I still get a big delay in my UI unless doing it like @Daiz.

Any reason the samples look like they do?

@shuding shuding added the good first issue Good for newcomers label Jun 7, 2020
@shuding
Copy link
Member

shuding commented Jun 7, 2020

This is a nice-to-have feature! I personally like the second API more (the 3rd argument can be a function or a promise):

// wait until the Promise from patchData resolves before triggering revalidate
mutate(path, { ...data, patch }, patchData(patch))

// async function
mutate(path, { ...data, patch }, async () => { return patchedData })

// no revalidation
mutate(path, { ...data, patch }, patchData(patch), false)

@johot
Copy link

johot commented Jun 7, 2020

Looks great @shuding !

But I have to ask, what is the reason for you guys mentioning this in the Readme:

Mutation and Post Request
In many cases, applying local mutations to data is a good way to make changes feel faster — no need to wait for the remote source of data.

With mutate, you can update your local data programmatically, while revalidating and finally replace it with the latest data.

...and then only showing examples of first awaiting a post then mutating?

I also wonder why would you even need mutate at all if not doing like suggested above? In a case where you still wait for a POST etc to get through couldn't one simply call swr.revalidate (after the POST) and just skip the mutate step?

Thank you your awesome work!

@shuding
Copy link
Member

shuding commented Jun 7, 2020

@johot yeah the example in the readme isn't good enough. It should first do a local mutation before sending the post request.

couldn't one simply call swr.revalidate (after the POST) and just skip the mutate step?

However the mutate in the example can still save one round trip, because revalidation takes time:

  1. post request
  2. local mutate + start revalidate // → data is updated
  3. finish revalidating

If one simply call swr.revalidate, it will be:

  1. post request
  2. revalidate
  3. finish revalidating // → data is updated

@Daiz
Copy link
Author

Daiz commented Jun 7, 2020

I personally like the second API more

@shuding The two suggestion aren't mutually exclusive, and would serve different purposes depending on how your server API works.

First scenario: patchData returns full up-to-date data

What you'd currently write with SWR:

mutate(path, { ...data, patch }, false); // mutate immediately locally, don't revalidate
mutate(path, patchData(patch), false); // update with API-backed result from patchData

How it could be simplified into a single mutate call with the addition of accepting { initialData: T } as an option for the third argument:

mutate(path, patchData(patch), { initialData: { ...data, patch });

Second scenario: patchData doesn't return full data

What you'd currently write with SWR:

mutate(path, { ...data, patch }, false); // mutate immediately, don't revalidate
await patchData(patch); // await the actual API call
trigger(path); // trigger revalidation afterwards

How it could be simplified into a single mutate call with the addition of accepting Promise<any> as an option for the third argument (where the promise resolving effectively acts as trigger(path)):

mutate(path, { ...data, patch }, patchData(patch));

It's entirely possible to implement both of these (you'd just need the sufficient overrides as well as an implementation that accepts boolean | { initialData: T } | Promise<any> as the third argument) and I could definitely take a stab at doing it myself after we have a conclusion on the API details here.

@johot
Copy link

johot commented Jun 8, 2020

@shuding Thank you for the explanation, now ofc it makes sense to me that you would still need to call mutate yourself since swr needs an extra roundtrip to the server which would cause further delays.

@Daiz @shuding
I definately think you should go for you suggested API of:

mutate(path, { ...data, patch }, patchData(patch));

and

mutate(path, { ...data, patch }, patchData(patch), false);

if you don't need to revalidate.

Maybe skip the "First scenario" API @Daiz as it might be harder to understand what is going on and relying to much on how the backend works?

@Daiz
Copy link
Author

Daiz commented Jun 8, 2020

relying to much on how the backend works?

@johot mutate already supports and documents how it can be used based on how the backend works (and even specifically calls out two scenarios of "returns full data" and "returns partial data"), so extending on that would be a pretty logical thing to do.

@Kerumen
Copy link
Contributor

Kerumen commented Dec 30, 2020

@shuding What's the progress on this subject? I'm very interested for this new API of mutate. Thanks!

@shuding
Copy link
Member

shuding commented Dec 26, 2021

Working on #1745 to support this case. 👍

tomkit added a commit to supaglue-labs/supaglue that referenced this issue Mar 11, 2023
- Prisma migration for `Integration`: adds `is_enabled` and makes
`config` optional
- Use Nextjs `useSWR`'s `mutate` to fetch and make updates
- Get creating, updating, deleting Integrations working
- Update openapi spec for creating/updating integrations

SWR related reading for using mutating local and remote state:
- vercel/swr#157
- vercel/swr#1745
- https://github.com/vercel/swr-site/pull/202/files

Other:
- Remove extra /dashboard page, use root instead for dashboard
- Use `Card` for `MetricCard` rather than `Grid`
- Rename `Active` tab to `Installed`
- Rename `Application 1` to `Your application`
- others
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants