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

Hook for triggering validations manually (passive mode) #1262

Closed
wants to merge 23 commits into from
Closed

Conversation

shuding
Copy link
Member

@shuding shuding commented Jul 2, 2021

Motivation

There are a lot of existing discussions and feature requests about doing requests manually in SWR (e.g. #93, #116, #550, #1082), and we do see some valid use cases for it. In short, we need something that:

  • Do not revalidate automatically (mount, focus, reconnect, retry...)
  • Return a callback function for triggering the validation manually
  • Backfill the request response to the cache*

*: Potentially we can have a switch to turn on/off this step.

One option is to have a new hook, because the arguments would be slightly different from the useSWR hook. Since we now have better abstractions to build such a hook, namely middlewares and revalidateWhenStale, this PR is such an attempt.

useSWRTrigger

import useSWRTrigger from 'swr/trigger'

// fetcher impl.
function getData(url, token, opts) { ... }

// normal SWR API, but it will never start the request
const { data, error, mutate, trigger } = useSWRTrigger('/api/user', getData, { onError, onSuccess })

// trigger it manually, args will be passed to the fetcher (after the key arg/args)
trigger('myToken', myOptions)

// wait until it's done
await trigger('myToken', myOptions)

The trigger() call will still reflect the hook-returned data and error with a re-render, but that's more declarative.

To-dos

  • Finalize the name of the hook.
  • Sometimes, mutation APIs (POST, PUT, DELETE) return different payload from the GET API. It might be an issue if we write back to the cache with the same key. This behavior needs to be reconsidered.
  • If the mutation APIs' results cannot be cached, that normally means that we need to revalidate those keys again if there's a useSWR hook with the same key. This can be another option to consider.
  • Ensure callbacks and isValidating state are working as expected.
  • Ensure with a test case that under trigger mode there's no deduping.
  • Ensure with a test case that trigger prevents race conditions with normal GET requests.
  • (docs) Have a specific list of options, that can be used in useSWRTrigger and ignore the others.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 2, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit be647cc:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
SWR-SSR Configuration

@shuding shuding added this to the 1.0 milestone Jul 19, 2021
@shuding shuding changed the title Add passive middleware Hook for triggering validations manually (passive mode) Aug 8, 2021
trigger/index.ts Outdated
if (!fetcher) throw new Error('Missing SWR fetcher.')

// Mutate the SWR data and return the result.
return swr.mutate(
Copy link

@giuseppeg giuseppeg Aug 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE:

Sometimes, mutation APIs (POST, PUT, DELETE) return different payload from the GET API. It might be an issue if we write back to the cache with the same key. This behavior needs to be reconsidered.

When the user is not interested in updating the global cache for key they can use the fetcher right away i.e. they won't need useSWRTrigger right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But I'm also thinking about an option to prevent updating the cache, but then useSWRTrigger will have nothing "SWR". Haven’t decided yet.

trigger/index.ts Outdated Show resolved Hide resolved
@huozhi huozhi modified the milestones: 1.0, backlog Aug 17, 2021
@shuding
Copy link
Member Author

shuding commented Aug 19, 2021

This API needs more discussions, will keep it on hold for 1.0.

@huozhi huozhi modified the milestones: backlog, 1.1 Aug 28, 2021
@shuding
Copy link
Member Author

shuding commented Sep 8, 2021

Some thoughts:

  • Passive hooks still need to have a key specified, even if it's not used by that request. Remote mutations are associated with normal GET requests, we need the key to avoid race conditions and do cache write back.
  • Each passive hook needs to have its own states (data, error, isValidating), which won't be shared with useSWR or other passive hooks with the same key.
  • Error retry should be turned off by default as remote mutations will be more likely to get 4xx errors than GET requests, and retrying won't help.
  • There should be a way to reset/clear the state as the request can be triggered again, and it is not idempotent like GET requests.
  • There should be an option to enable/disable writing back the value into the cache, as well as triggering a revalidation after it ends.

@giuseppeg
Copy link

Some thoughts:

  • Passive hooks still need to have a key specified, even if it's not used by that request. Remote mutations are associated with normal GET requests, we need the key to avoid race conditions and do cache write back.
  • Each passive hook needs to have its own states (data, error, isValidating), which won't be shared with useSWR or other passive hooks with the same key.

The solution that I am currently using (https://twitter.com/giuseppegurgone/status/1427576808204034061) is close to this except that I don't use a key and revalidate but mutate programmatically.

I think that triggering a revalidation (a GET request) when the remote mutation is complete is a good option although it could be redundant work, lead to higher consumption of resources since the mutation could already return the fresh data.

@shuding shuding mentioned this pull request Sep 12, 2021
12 tasks
@shuding shuding closed this Sep 12, 2021
@shuding shuding deleted the passive branch September 12, 2021 15:03
@shuding
Copy link
Member Author

shuding commented Sep 12, 2021

Continuing this feature in #1450.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants