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

fix: delay revalidation if a key is already active and has error #2338

Merged
merged 7 commits into from Jan 24, 2023

Conversation

promer94
Copy link
Collaborator

@promer94 promer94 commented Dec 26, 2022

close #2336

The reason for #2336 is that

every time a revalidation is finished, a new swr hook will mount and trigger a new revalidation.

isValidating will also cause similar problem here.

https://codesandbox.io/s/isvalidating-infinite-render-xgwyyz

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 26, 2022

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 fe8a088:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
SWR-SSR Configuration
isValidating-infinite-render PR
frosty-silence-k2hgx5 Issue #2336

core/use-swr.ts Outdated Show resolved Hide resolved
core/use-swr.ts Outdated Show resolved Hide resolved
@promer94 promer94 requested a review from koba04 December 26, 2022 18:16
core/use-swr.ts Show resolved Hide resolved
@promer94
Copy link
Collaborator Author

promer94 commented Jan 9, 2023

@promer94 Thank you! I understand the intention 👍

Let me share another solution I came up with. I feel the issue is caused by an incorrect isLoading state on an error, and I'd say isLoading should be true only when both data and error are undefined.

swr/core/use-swr.ts

Lines 302 to 304 in 3cec4bd

if (isUndefined(getCache().data)) {
initialState.isLoading = true
}

should be

      if (isUndefined(getCache().data) && isUndefined(getCache().error)) {
        initialState.isLoading = true
      }

What do you think?

After reviewing #1928, I think it would be batter to discuss this change in a spreate pr.

@promer94 promer94 requested a review from koba04 January 9, 2023 05:03
Copy link
Collaborator

@koba04 koba04 left a comment

Choose a reason for hiding this comment

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

I'd say it's a bit tricky checking hasValidator, but as @promer94 mentioned at #2338 (comment), this is a way to fix the problem without changing existing behaviors. LGTM 👍

@promer94 promer94 merged commit b30d808 into vercel:main Jan 24, 2023
@promer94 promer94 deleted the delay-revalidation branch January 24, 2023 16:19
renovate bot added a commit to Unleash/unleash that referenced this pull request Feb 12, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [swr](https://swr.vercel.app)
([source](https://togithub.com/vercel/swr)) | [`2.0.0` ->
`2.0.3`](https://renovatebot.com/diffs/npm/swr/2.0.0/2.0.3) |
[![age](https://badges.renovateapi.com/packages/npm/swr/2.0.3/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/swr/2.0.3/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/swr/2.0.3/compatibility-slim/2.0.0)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/swr/2.0.3/confidence-slim/2.0.0)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>vercel/swr</summary>

### [`v2.0.3`](https://togithub.com/vercel/swr/releases/tag/v2.0.3)

[Compare
Source](https://togithub.com/vercel/swr/compare/v2.0.2...v2.0.3)

#### Patches

- fix: parameters passed to subscribe is inconsistent with its
declaration by [@&#8203;hong24](https://togithub.com/hong24) in
[vercel/swr#2353
- fix: when inifinite key changes, should use cached pagesize instead of
initialSize by [@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2386
- fix: make compare function only compare data
([#&#8203;2375](https://togithub.com/vercel/swr/issues/2375)) by
[@&#8203;Retrospection](https://togithub.com/Retrospection) and
[@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2387
[vercel/swr#2389
- fix: initial loading state should be false with revalidation is
disabed by [@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2382
- fix: Ensure hydration matches the SSR result during streaming by
[@&#8203;shuding](https://togithub.com/shuding) in
[vercel/swr#2391

#### Chores

- ci: only run release when tags pushed by
[@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2388
- unify `-` characters in README.md by
[@&#8203;JanKaifer](https://togithub.com/JanKaifer) in
[vercel/swr#2334

#### New Contributors

- [@&#8203;hong24](https://togithub.com/hong24) made their first
contribution in
[vercel/swr#2353
- [@&#8203;Retrospection](https://togithub.com/Retrospection) made their
first contribution in
[vercel/swr#2387
- [@&#8203;JanKaifer](https://togithub.com/JanKaifer) made their first
contribution in
[vercel/swr#2334

**Full Changelog**:
vercel/swr@v2.0.2...v2.0.3

### [`v2.0.2`](https://togithub.com/vercel/swr/releases/tag/v2.0.2)

[Compare
Source](https://togithub.com/vercel/swr/compare/v2.0.1...v2.0.2)

#### Patches

- fix: delay revalidation if a key is already active and has error by
[@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2338
- fix: createCacheHelper type by
[@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2365

#### Chores

- release workflow by [@&#8203;huozhi](https://togithub.com/huozhi) in
[vercel/swr#2374,
[vercel/swr#2381,
[vercel/swr#2383,
[vercel/swr#2384,
[vercel/swr#2385

**Full Changelog**:
vercel/swr@v2.0.1...v2.0.2

### [`v2.0.1`](https://togithub.com/vercel/swr/releases/tag/v2.0.1)

[Compare Source](https://togithub.com/vercel/swr/compare/2.0.0...v2.0.1)

In this patch release, we majorly improved typing support and SWR can
infer the types based on the configuration.

- When the `suspense` option is `true`, the returned `data` will exclude
`undefined` and the `isLoading` will always be `false`.
- When the `fallbackData` option is provided, the returned `data` will
be the same type of `fallbackData`, and the `isLoading` will always be
`false`.

Here's a demo for it:


https://user-images.githubusercontent.com/4800338/213288879-ffbeeeb5-aca5-4a65-8229-69fbf9b0bf70.mp4

#### What's Changed

- types: make FullConfiguration generic by
[@&#8203;huozhi](https://togithub.com/huozhi) in
[vercel/swr#2273
- type: enforce array literal as tuple by
[@&#8203;promer94](https://togithub.com/promer94) in
[vercel/swr#2283
- Improve type test by [@&#8203;promer94](https://togithub.com/promer94)
in
[vercel/swr#2289
- update bunchee to remove the extra target cli option by
[@&#8203;huozhi](https://togithub.com/huozhi) in
[vercel/swr#2298
- fix: useSWRMutation should always use the latest fetcher by
[@&#8203;koba04](https://togithub.com/koba04) in
[vercel/swr#2290
- types: conditional swr response by
[@&#8203;huozhi](https://togithub.com/huozhi) in
[vercel/swr#2301
- fix: types properites in exports fields by
[@&#8203;koba04](https://togithub.com/koba04) in
[vercel/swr#2341
- Update license year by
[@&#8203;IcedMonk](https://togithub.com/IcedMonk) in
[vercel/swr#2344
- feat: add all mutate options support to useSWRInfinite by
[@&#8203;koba04](https://togithub.com/koba04) in
[vercel/swr#2350
- ci: add publint to check packaging on CI by
[@&#8203;koba04](https://togithub.com/koba04) in
[vercel/swr#2363

#### New Contributors

- [@&#8203;IcedMonk](https://togithub.com/IcedMonk) made their first
contribution in
[vercel/swr#2344

**Full Changelog**: vercel/swr@2.0.0...v2.0.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/Unleash/unleash).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMzAuMSIsInVwZGF0ZWRJblZlciI6IjM0LjEzMC4xIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

Loading state and UI lead to Infinite retry
2 participants