Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ vitest.config.ts.*

# Yalc
yalc.lock
.yalc/
.yalc/

# ai
.ai
AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import {
CATEGORIES,
CONSENT_ADVERTISING_MAX_AGE,
CONSENT_MAX_AGE,
COOKIES_OPTIONS,
COOKIE_PREFIX,
COOKIES_OPTIONS,
HASH_COOKIE,
} from '../constants'
import { uniq } from '../helpers/array'
Expand Down
39 changes: 39 additions & 0 deletions packages/use-dataloader/.changeset/green-trees-impress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
"@scaleway/use-dataloader": major
---

## Changes

### Breaking Changes

- Changed `isLoading` behavior to be `true` only during the initial fetch when there's no cached data
- `isLoading` is now `false` during subsequent fetches, even when `isFetching` is `true`

### New Features

- Added distinction between initial loading (`isLoading`) and ongoing fetching (`isFetching`)
- `isLoading` is `true` only when there is no cache data and we're fetching for the first time
- `isFetching` remains `true` during any active request (initial or subsequent)

### Migration Guide

If you were relying on `isLoading` to detect all fetching states (including refreshes), you should now use `isFetching` instead:

```javascript
// Before - this would be true for both initial load and refreshes
if (result.isLoading) {
// Show loading spinner
}

// After - use isFetching to detect all fetching states
if (result.isFetching) {
// Show loading spinner for both initial load and refreshes
}

// Use isLoading only if you want to distinguish initial loads
if (result.isLoading) {
// Show full-page loading for initial load only
} else if (result.isFetching) {
// Show refresh indicator for subsequent fetches
}
```
26 changes: 17 additions & 9 deletions packages/use-dataloader/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,19 +118,19 @@ const fakePromise = () =>

function MyComponent() {
// Use a key if you want to persist data in the DataLoaderProvider cache
const { data, isLoading, isSuccess, isError, error } = useDataLoader(
const { data, isLoading, isFetching, isSuccess, isError, error } = useDataLoader(
'cache-key',
fakePromise,
)

// This is the first time we load the data
if (isLoading && !data) {
return <div>Loading...</div>
return <div>Loading initial data...</div>
}

// This happen when you already load the data but want to reload it
if (isLoading && data) {
return <div>Reloading...</div>
// This happen when you already loaded the data but want to reload it
if (isFetching && data) {
return <div>Refreshing...</div>
}

// Will be true when the promise is resolved
Expand Down Expand Up @@ -162,14 +162,19 @@ const fakePromise = () =>

function MyComponentThatUseDataLoader({key}) {
// Use a key if you want to persist data in the DataLoaderProvider cache
const { data, isLoading, isSuccess, isError, error } = useDataLoader(
const { data, isLoading, isFetching, isSuccess, isError, error } = useDataLoader(
key,
fakePromise,
)

// Will be true during the promise
// Will be true during the initial load
if (isLoading) {
return <div>Loading...</div>
return <div>Loading initial data...</div>
}

// Will be true during any active request (initial or subsequent)
if (isFetching) {
return <div>Refreshing...</div>
}

// Will be true when the promise is resolved
Expand Down Expand Up @@ -236,10 +241,13 @@ const useDataLoader = (
)
```

The hook returns an object with the following properties:

| Property | Description |
| :----------: | :------------------------------------------------------------------------------------------------------------------------------------------: |
| isIdle | `true` if the request is not launched |
| isLoading | `true` if the request is launched **or** enabled is `true` and isIdle is `true` |
| isLoading | `true` only during the initial fetch when there's no cached data |
| isFetching | `true` when there is an active request in progress (initial or subsequent) |
| isSuccess | `true`if the request finished successfully |
| isError | `true` if the request throw an error |
| isPolling | `true` if the request if `enabled` is true, `pollingInterval` is defined and the status is `isLoading`,`isSuccess` or during the first fetch |
Expand Down
91 changes: 78 additions & 13 deletions packages/use-dataloader/src/__tests__/useDataLoader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ describe('useDataLoader', () => {
expect(result.current.isLoading).toBe(false)
result.current.reload().catch(() => null)
result.current.reload().catch(() => null)
await waitFor(() => expect(result.current.isLoading).toBe(true))
await waitFor(() => expect(result.current.isFetching).toBe(true))
expect(result.current.data).toBe(true)
await waitFor(() => expect(result.current.isSuccess).toBe(true))
expect(result.current.data).toBe(true)
Expand Down Expand Up @@ -324,20 +324,21 @@ describe('useDataLoader', () => {
)
expect(result.current.data).toBe(undefined)
expect(result.current.isPolling).toBe(true)
expect(result.current.isLoading).toBe(true)
expect(result.current.isFetching).toBe(true)
expect(pollingProps.method).toBeCalledTimes(1)
await waitFor(() => expect(result.current.isSuccess).toBe(true))
expect(result.current.data).toBe(true)
expect(result.current.isSuccess).toBe(true)
expect(result.current.isPolling).toBe(true)
await waitFor(() => expect(result.current.isLoading).toBe(true))
await waitFor(() => expect(result.current.isFetching).toBe(true))
expect(pollingProps.method).toBeCalledTimes(2)
expect(result.current.isPolling).toBe(true)

await waitFor(() => expect(result.current.isSuccess).toBe(true))
expect(result.current.data).toBe(true)
expect(result.current.isPolling).toBe(true)
expect(result.current.isSuccess).toBe(true)
expect(result.current.isLoading).toBe(false)
expect(result.current.isFetching).toBe(false)
rerender({
...pollingProps,
config: {
Expand All @@ -348,13 +349,13 @@ describe('useDataLoader', () => {
expect(result.current.data).toBe(true)
expect(result.current.isPolling).toBe(true)
expect(result.current.isSuccess).toBe(true)
expect(result.current.isLoading).toBe(false)
await waitFor(() => expect(result.current.isLoading).toBe(true))
expect(result.current.isFetching).toBe(false)
await waitFor(() => expect(result.current.isFetching).toBe(true))
expect(result.current.isSuccess).toBe(false)
await waitFor(() => expect(result.current.isSuccess).toBe(true))
expect(method2).toBeCalledTimes(1)
expect(result.current.isPolling).toBe(true)
expect(result.current.isLoading).toBe(false)
expect(result.current.isSuccess).toBe(true)
expect(result.current.isFetching).toBe(false)
expect(result.current.data).toBe(2)

rerender({
Expand All @@ -364,7 +365,7 @@ describe('useDataLoader', () => {
},
method: method2,
})
await waitFor(() => expect(result.current.isLoading).toBe(true))
await waitFor(() => expect(result.current.isFetching).toBe(true))
expect(result.current.data).toBe(2)
expect(result.current.isPolling).toBe(true)
expect(result.current.isSuccess).toBe(false)
Expand Down Expand Up @@ -752,7 +753,7 @@ describe('useDataLoader', () => {
expect(result.current[0]?.data).toBe(true)

result.current[1]?.reload().catch(() => null)
await waitFor(() => expect(result.current[1]?.isLoading).toBe(true))
await waitFor(() => expect(result.current[1]?.isFetching).toBe(true))
expect(result.current[1]?.data).toBe(true)

await waitFor(() => expect(result.current[1]?.isSuccess).toBe(true))
Expand Down Expand Up @@ -801,7 +802,7 @@ describe('useDataLoader', () => {
expect(mockedFn).toBeCalledTimes(1)

result.current[1].reloadAll().catch(() => null)
await waitFor(() => expect(result.current[0].isLoading).toBe(true))
await waitFor(() => expect(result.current[0].isFetching).toBe(true))
expect(result.current[0].data).toBe(true)
expect(Object.values(result.current[1].getReloads()).length).toBe(1)

Expand Down Expand Up @@ -883,8 +884,8 @@ describe('useDataLoader', () => {
await waitFor(() => expect(result.current[0]?.isSuccess).toBe(true))
testingProps.config2.enabled = true
rerender(testingProps)
await waitFor(() => expect(result.current[0]?.isLoading).toBe(true))
await waitFor(() => expect(result.current[1]?.isLoading).toBe(true))
await waitFor(() => expect(result.current[0]?.isFetching).toBe(true))
await waitFor(() => expect(result.current[1]?.isFetching).toBe(true))
expect(testingProps.method).toBeCalledTimes(2)
expect(result.current[0]?.data).toBe(true)
expect(result.current[0]?.previousData).toBe(undefined)
Expand All @@ -893,4 +894,68 @@ describe('useDataLoader', () => {
await waitFor(() => expect(result.current[0]?.isSuccess).toBe(true))
await waitFor(() => expect(result.current[1]?.isSuccess).toBe(true))
})

test('should differentiate between isLoading and isFetching', async () => {
let resolveIt = false
const method = vi.fn(() => {
const promiseFn = () =>
new Promise(resolve => {
setInterval(() => {
if (resolveIt) {
resolve({ id: 1, name: 'test' })
}
}, PROMISE_TIMEOUT)
})

return promiseFn()
})

const testProps = {
config: {
enabled: true,
},
key: 'test-isLoading-vs-isFetching',
method,
}

const { result } = renderHook(
props => useDataLoader(props.key, props.method, props.config),
{
initialProps: testProps,
wrapper,
},
)

// Initially, isLoading should be true (first load with no cache)
expect(result.current.isLoading).toBe(true)
expect(result.current.isFetching).toBe(true)
expect(result.current.data).toBe(undefined)

// Resolve the first request
resolveIt = true
await waitFor(() => expect(result.current.isSuccess).toBe(true))

// After first load, both should be false
expect(result.current.isLoading).toBe(false)
expect(result.current.isFetching).toBe(false)
expect(result.current.data).toEqual({ id: 1, name: 'test' })

// Trigger a reload
resolveIt = false
result.current.reload().catch(() => null)

// During reload, isLoading should be false (we have cached data) but isFetching should be true
await waitFor(() => expect(result.current.isFetching).toBe(true))
expect(result.current.isLoading).toBe(false)
expect(result.current.data).toEqual({ id: 1, name: 'test' }) // Still have cached data

// Resolve the reload
resolveIt = true
await waitFor(() => expect(result.current.isSuccess).toBe(true))

// After reload, both should be false again
expect(result.current.isLoading).toBe(false)
expect(result.current.isFetching).toBe(false)
expect(result.current.data).toEqual({ id: 1, name: 'test' })
})
})
Loading
Loading