Skip to content

feat: improve dataloader refetch handling #608

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 24, 2022
Merged

feat: improve dataloader refetch handling #608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion packages/use-dataloader/src/DataLoaderProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type CachedData = Record<string, unknown>
type Reloads = Record<string, () => Promise<void | unknown>>

type UseDataLoaderInitializerArgs<T = unknown> = {
enabled?: boolean
status?: StatusEnum
method: () => PromiseType<T>
pollingInterval?: number
Expand Down
52 changes: 26 additions & 26 deletions packages/use-dataloader/src/__tests__/DataLoaderProvider.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ describe('DataLoaderProvider', () => {
expect(Object.values(result.current.getReloads()).length).toBe(1)
const testRequest = result.current.getRequest(TEST_KEY)
expect(testRequest).toBeDefined()
expect(testRequest?.status).toBe(StatusEnum.IDLE)
expect(testRequest.status).toBe(StatusEnum.IDLE)
act(() => {
// launch should never throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
testRequest?.launch()
testRequest.load()
})
expect(method).toBeCalledTimes(1)
expect(testRequest.status).toBe(StatusEnum.LOADING)
Expand Down Expand Up @@ -92,11 +92,11 @@ describe('DataLoaderProvider', () => {
const testRequest = result.current.getRequest(TEST_KEY)
expect(Object.values(result.current.getReloads()).length).toBe(1)
expect(testRequest).toBeDefined()
expect(testRequest?.status).toBe(StatusEnum.IDLE)
expect(testRequest.status).toBe(StatusEnum.IDLE)
act(() => {
// launch should never throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
testRequest?.launch()
testRequest.load()
})
expect(method).toBeCalledTimes(1)
await waitFor(() => expect(testRequest.getData()).toBe(true))
Expand All @@ -112,14 +112,14 @@ describe('DataLoaderProvider', () => {
wrapper,
})
act(() => {
result.current.getOrAddRequest(TEST_KEY, {
enabled: true,
const request = result.current.getOrAddRequest(TEST_KEY, {
method,
})
result.current.getOrAddRequest(TEST_KEY, {
enabled: true,
method,
})
// eslint-disable-next-line @typescript-eslint/no-floating-promises
request.load()
})
expect(method).toBeCalledTimes(1)
await waitFor(() =>
Expand Down Expand Up @@ -198,25 +198,25 @@ describe('DataLoaderProvider', () => {
wrapper: wrapperWith2ConcurrentRequests,
})
act(() => {
result.current.addRequest(TEST_KEY, {
enabled: true,
method,
})
result.current.addRequest(`${TEST_KEY}-2`, {
enabled: true,
method,
})
result.current.addRequest(`${TEST_KEY}-3`, {
enabled: true,
method,
})
result.current.addRequest(`${TEST_KEY}-4`, {
enabled: true,
method,
})
result.current.addRequest(`${TEST_KEY}-5`, {
enabled: true,
method,
;[
result.current.addRequest(TEST_KEY, {
method,
}),
result.current.addRequest(`${TEST_KEY}-2`, {
method,
}),
result.current.addRequest(`${TEST_KEY}-3`, {
method,
}),
result.current.addRequest(`${TEST_KEY}-4`, {
method,
}),
result.current.addRequest(`${TEST_KEY}-5`, {
method,
}),
].forEach(request => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
request.load()
})
})
expect(method).toBeCalledTimes(2)
Expand Down
110 changes: 43 additions & 67 deletions packages/use-dataloader/src/__tests__/dataloader.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const fakeErrorPromise = () =>
})

describe('Dataloader class', () => {
test('should create instance not enabled then load then destroy', async () => {
test('should create instance then load then destroy', async () => {
const method = jest.fn(fakeSuccessPromise)
const instance = new DataLoader({
key: 'test',
Expand All @@ -33,90 +33,71 @@ describe('Dataloader class', () => {
expect(instance.status).toBe(StatusEnum.IDLE)
expect(method).toBeCalledTimes(0)
await instance.load()
expect(instance.status).toBe(StatusEnum.SUCCESS)
expect(method).toBeCalledTimes(1)
await instance.destroy()
instance.clearData()
})

test('should create instance enabled with cancel', async () => {
test('should create instance with cancel', async () => {
const notify = jest.fn()
const method = jest.fn(fakeSuccessPromise)
const instance = new DataLoader({
enabled: true,
key: 'test',
method,
})
instance.addObserver(notify)
expect(instance.status).toBe(StatusEnum.LOADING)
expect(instance.getData()).toBe(undefined)
expect(notify).toBeCalledTimes(0)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
instance.load()
expect(method).toBeCalledTimes(1)
await instance.cancel()
await new Promise(resolve => {
setTimeout(() => {
expect(notify).toBeCalledTimes(1)
resolve(true)
}, PROMISE_TIMEOUT)
})
expect(notify).toBeCalledTimes(1)
expect(instance.getData()).toBe(undefined)
instance.clearData()
})

test('should create instance enabled without cancel', async () => {
test('should create instance without cancel', async () => {
const notify = jest.fn()
const method = jest.fn(fakeSuccessPromise)
const instance = new DataLoader({
enabled: true,
key: 'test',
method,
})
instance.addObserver(notify)
expect(instance.status).toBe(StatusEnum.LOADING)
expect(notify).toBeCalledTimes(0)
expect(instance.status).toBe(StatusEnum.IDLE)
await instance.load()
expect(notify).toBeCalledTimes(2)
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(() => {
expect(notify).toBeCalledTimes(1)
resolve(true)
}, PROMISE_TIMEOUT)
})
expect(instance.getData()).toBe(true)
instance.getObserversCount()
instance.removeObserver(notify)
instance.removeObserver(notify)
instance.clearData()
})

test('should create instance enabled with null data', async () => {
test('should create instance with null data', async () => {
const method = jest.fn(fakeNullPromise)
const instance = new DataLoader({
enabled: true,
key: 'test',
method,
})
expect(instance.status).toBe(StatusEnum.LOADING)
expect(instance.status).toBe(StatusEnum.IDLE)
await instance.load()
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(() => {
resolve(true)
}, PROMISE_TIMEOUT)
})
expect(instance.getData()).toBe(undefined)
})
test('should create instance enabled with undefined data', async () => {
test('should create instance with undefined data', async () => {
const method = jest.fn(fakeUndefinedPromise)
const instance = new DataLoader({
enabled: true,
key: 'test',
method,
})
expect(instance.status).toBe(StatusEnum.LOADING)
expect(instance.status).toBe(StatusEnum.IDLE)
await instance.load()
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(() => {
resolve(true)
}, PROMISE_TIMEOUT)
})
expect(instance.getData()).toBe(undefined)
})

Expand All @@ -129,8 +110,8 @@ describe('Dataloader class', () => {
})
instance.addOnCancelListener(onCancel)
instance.addOnCancelListener(onCancel)
// eslint-disable-next-line no-void
void instance.load()
// eslint-disable-next-line @typescript-eslint/no-floating-promises
instance.load()
await instance.cancel()
expect(onCancel).toBeCalledTimes(1)
instance.removeOnCancelListener(onCancel)
Expand All @@ -146,8 +127,8 @@ describe('Dataloader class', () => {
})
instance.addOnCancelListener(onCancel)
instance.addOnCancelListener(onCancel)
// eslint-disable-next-line no-void
void instance.load()
// eslint-disable-next-line @typescript-eslint/no-floating-promises
instance.load()
await instance.cancel()
expect(onCancel).toBeCalledTimes(1)
instance.removeOnCancelListener(onCancel)
Expand Down Expand Up @@ -190,30 +171,31 @@ describe('Dataloader class', () => {
const instance = new DataLoader({
key: 'test',
method,
pollingInterval: PROMISE_TIMEOUT * 2,
pollingInterval: PROMISE_TIMEOUT,
})
await instance.load()
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT * 3)
await waitForExpect(() => {
expect(method).toBeCalledTimes(2)
})
expect(method).toBeCalledTimes(2)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT * 3)
await waitForExpect(() => {
expect(method).toBeCalledTimes(3)
})
expect(method).toBeCalledTimes(3)
await instance.load()
await instance.load()
await waitForExpect(() => {
expect(method).toBeCalledTimes(4)
})
await waitForExpect(() => {
expect(method).toBeCalledTimes(5)
})
await instance.load()
await instance.load()
await instance.load(true)
await waitForExpect(() => {
expect(method).toBeCalledTimes(6)
})
instance.setPollingInterval(PROMISE_TIMEOUT * 4)
instance.setPollingInterval(PROMISE_TIMEOUT * 2)
await instance.destroy()
})

Expand All @@ -227,24 +209,22 @@ describe('Dataloader class', () => {
})
await instance.load()
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT * 3)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
await instance.load()
await waitForExpect(() => {
expect(method).toBeCalledTimes(2)
})
expect(method).toBeCalledTimes(2)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT * 3)
await waitForExpect(() => {
expect(method).toBeCalledTimes(3)
})
expect(method).toBeCalledTimes(3)
await instance.load()
await instance.load()
await waitForExpect(() => {
expect(method).toBeCalledTimes(4)
})
await instance.load()
await instance.load()
await instance.load(true)
await waitForExpect(() => {
expect(method).toBeCalledTimes(6)
expect(method).toBeCalledTimes(5)
})
instance.setPollingInterval(PROMISE_TIMEOUT * 4)
await instance.destroy()
Expand Down Expand Up @@ -272,24 +252,20 @@ describe('Dataloader class', () => {
const method = jest.fn(fakeSuccessPromise)
const onSuccess = jest.fn()
const instance = new DataLoader({
enabled: true,
key: 'test',
maxDataLifetime: PROMISE_TIMEOUT,
maxDataLifetime: PROMISE_TIMEOUT * 3,
method,
})
instance.addOnSuccessListener(onSuccess)
expect(instance.status).toBe(StatusEnum.LOADING)
expect(instance.status).toBe(StatusEnum.IDLE)
await instance.load()
expect(method).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT)
})
expect(onSuccess).toBeCalledTimes(1)
await instance.load()
expect(method).toBeCalledTimes(1)
expect(onSuccess).toBeCalledTimes(1)
await new Promise(resolve => {
setTimeout(resolve, PROMISE_TIMEOUT * 2)
})
// Wait until data is outdated
await waitForExpect(() => expect(instance.isDataOutdated).toBeTruthy())
await instance.load()
expect(method).toBeCalledTimes(2)
expect(onSuccess).toBeCalledTimes(2)
Expand All @@ -300,11 +276,11 @@ describe('Dataloader class', () => {
DataLoader.maxConcurrent = 2
for (let i = 0; i < 5; i += 1) {
const instance = new DataLoader({
enabled: true,
key: `test-${i}`,
method,
})
expect(instance.status).toBe(StatusEnum.LOADING)
// eslint-disable-next-line @typescript-eslint/no-floating-promises
instance.load()
}
// Because wait for setTimeout tryLaunch in dataloader.ts
await waitForExpect(() => {
Expand Down
Loading