Skip to content

Mutate with custom updater when no useSWR seems to have wrong state #832

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

Closed
AyWa opened this issue Dec 19, 2020 · 8 comments · Fixed by #833
Closed

Mutate with custom updater when no useSWR seems to have wrong state #832

AyWa opened this issue Dec 19, 2020 · 8 comments · Fixed by #833
Labels
bug Something isn't working

Comments

@AyWa
Copy link

AyWa commented Dec 19, 2020

Bug report

Description / Observed Behavior

Simple case is:

  • a user action that will trigger a mutate on a key with a custom updater
  • after this action is done mount a component (or in a next js app push to router) that will use this key

-> the data that I see is not correct.

Expected Behavior

Expected behavior are:

  • if mutate is call and then a first useSWR is done on the same key, we should expect correct data -> request is correctly done, but data does not seems to be use / drop

Repo Steps / Code Example

if you run this example: click on the button you will see that request is done -> but only the mutate data is use.

https://codesandbox.io/s/clever-knuth-2z40o?file=/src/App.js:1128-1161

Additional Context

0.3.9

@huozhi
Copy link
Member

huozhi commented Dec 19, 2020

@AyWa if you display the button and DisplayData all the time, and change the last argument of mutate from true to false, you'll see:

  1. fetched data by useSWR is rendered at first
  2. click button
  3. the mutated data will replace the default data

from:

(after click) to:

is this what you expected?

@AyWa
Copy link
Author

AyWa commented Dec 19, 2020

No this is not what I want to see. Sorry if I didnt explain well.
The behavior is:

  • I do not want to display the 2 component at the same time (because they could be different page) - > So the example given replicate that
  • I do not want to set the last argument to false (because I want to revalidate)

What I expect is:

  • user click button
  • mutate the key - > update data
  • mount a component that will use the key
  • so show fake
  • after api answers it should be updated

Does the flow is clear now? Let me know If my explanation is correct

@huozhi
Copy link
Member

huozhi commented Dec 19, 2020

I think the reason is that the when mutate is done, the revalidation is started, but the useSWR hook is not been called yet (DisplayData is not mounted). you might want to find a way to keep your hook there before displaying. like

<SWRConfig>
  <DisplayData visible={display} />
  {!display && <button /> // for mutation
const DisplayData = ({visible}) => {
   const {data} = useSWR(...)
   return visible && <div>....</div>
}

@promer94
Copy link
Collaborator

I think this is a bug which was introduced by #735

swr/src/use-swr.ts

Lines 445 to 463 in 7858407

// if there're other mutations(s), overlapped with the current revalidation:
// case 1:
// req------------------>res
// mutate------>end
// case 2:
// req------------>res
// mutate------>end
// case 3:
// req------------------>res
// mutate-------...---------->
// we have to ignore the revalidation result (res) because it's no longer fresh.
// meanwhile, a new revalidation should be triggered when the mutation ends.
(MUTATION_TS[key] &&
// case 1
(startAt <= MUTATION_TS[key] ||
// case 2
startAt <= MUTATION_END_TS[key] ||
// case 3
MUTATION_END_TS[key] === 0))

we ignored a case like this

// case 4: 
 //                                  req------------>res 
 //   mutate------>end 

@promer94
Copy link
Collaborator

Maybe we could reset the mutation time stamp to undefined if there is no updater in CACHE_REVALIDATORS ?

const updaters = CACHE_REVALIDATORS[key]
if (updaters) {
    const promises = []
    for (let i = 0; i < updaters.length; ++i) {
      promises.push(
        updaters[i](!!shouldRevalidate, data, error, undefined, i > 0)
      )
    }
    // return new updated value
    return Promise.all(promises).then(() => {
      if (error) throw error
      return cache.get(key)
    })
} else {
  MUTATION_TS[key] = undefined
  MUTATION_END_TS[key] = undefined
}

promer94 added a commit to promer94/swr that referenced this issue Dec 19, 2020
@promer94 promer94 mentioned this issue Dec 19, 2020
@shuding
Copy link
Member

shuding commented Dec 19, 2020

We can't ignore the revalidation result if it's this case (mutation ended before the revalidation starts):

// case 4: 
//                     req------------>res 
//   mutate------>end 

It sounds OK to keep the request?

And it's still not clear to me why it's related to #735... I'll check which case causes that 🤔

@shuding
Copy link
Member

shuding commented Dec 19, 2020

Commented here: #833, it was indeed a bug!

@shuding shuding added the bug Something isn't working label Dec 19, 2020
shuding pushed a commit that referenced this issue Dec 20, 2020
@AyWa
Copy link
Author

AyWa commented Dec 20, 2020

Thanks a lot for the fast investigation and fix !! You Rock 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants