Skip to content

Commit

Permalink
internal: Fix timing based test race conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
ntucker committed Dec 20, 2024
1 parent 07dce8f commit 8a2c8c1
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 44 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Endpoint, Entity } from '@data-client/endpoint';
import { CacheProvider } from '@data-client/react';
import { DataProvider as ExternalDataProvider } from '@data-client/react/redux';
import { jest } from '@jest/globals';
import { afterEach, jest } from '@jest/globals';
import {
OptimisticArticleResource,
ArticleResourceWithOtherListUrl,
Expand All @@ -17,7 +17,7 @@ import { useContext } from 'react';

import { makeRenderDataClient, act } from '../../../test';
import { StateContext } from '../context';
import { useCache, useController, useSuspense } from '../hooks';
import { useCache, useSuspense } from '../hooks';
import { useError } from '../hooks';
import {
payload,
Expand Down Expand Up @@ -446,6 +446,7 @@ describe.each([
});

it('should clear only earlier optimistic updates when a promise resolves', async () => {
jest.useFakeTimers({ legacyFakeTimers: false });
const params = { id: payload.id };
const { result, controller } = renderDataClient(
() => {
Expand All @@ -462,9 +463,7 @@ describe.each([
],
},
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 0));
});
jest.advanceTimersByTime(23);

const fetches: Promise<any>[] = [];
const resolves: ((v: any) => void)[] = [];
Expand Down Expand Up @@ -495,9 +494,7 @@ describe.each([
content: 'firstoptimistic',
}),
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 1));
});
jest.advanceTimersByTime(23);

// second optimistic
act(() => {
Expand All @@ -524,9 +521,7 @@ describe.each([
content: 'firstoptimistic',
}),
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 0));
});
jest.advanceTimersByTime(23);

// third optimistic
act(() => {
Expand Down Expand Up @@ -556,9 +551,7 @@ describe.each([
);

// resolve second request while first is in flight
act(() => {
setTimeout(() => resolves[1]({ ...payload, title: 'second' }), 1);
});
resolves[1]({ ...payload, title: 'second' });
await act(() => fetches[1]);

// first and second optimistic should be cleared with only third optimistic left to be layerd
Expand All @@ -573,16 +566,10 @@ describe.each([

// resolve the first fetch; but the second fetch happened after so we use it first since this is default 'first fetchedAt' behavior
// this can be solved by either canceling requests or having server send the total order
act(() => {
setTimeout(
() =>
resolves[0]({
...payload,
title: 'first',
content: 'first',
}),
1,
);
resolves[0]({
...payload,
title: 'first',
content: 'first',
});
await act(() => fetches[0]);
expect(result.current.article).toEqual(
Expand All @@ -592,9 +579,12 @@ describe.each([
tags: ['thirdoptimistic'],
}),
);
jest.useRealTimers();
await renderDataClient.allSettled();
});

it('should clear optimistic when server response resolves in order', async () => {
jest.useFakeTimers({ legacyFakeTimers: false });
const params = { id: payload.id };
const { result, controller } = renderDataClient(
() => {
Expand All @@ -610,9 +600,7 @@ describe.each([
],
},
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 1));
});
jest.advanceTimersByTime(23);

const fetches: Promise<any>[] = [];
const resolves: ((v: any) => void)[] = [];
Expand Down Expand Up @@ -643,9 +631,7 @@ describe.each([
content: 'firstoptimistic',
}),
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 0));
});
jest.advanceTimersByTime(23);

// second optimistic
act(() => {
Expand All @@ -672,9 +658,7 @@ describe.each([
content: 'firstoptimistic',
}),
);
await act(async () => {
await new Promise(resolve => setTimeout(resolve, 0));
});
jest.advanceTimersByTime(23);

// third optimistic
act(() => {
Expand Down Expand Up @@ -702,11 +686,10 @@ describe.each([
tags: ['thirdoptimistic'],
}),
);
jest.advanceTimersByTime(23);

// resolve first request
act(() => {
setTimeout(() => resolves[0]({ ...payload, content: 'first' }), 0);
});
resolves[0]({ ...payload, content: 'first' });
await act(() => fetches[0]);

// replace optimistic with response
Expand All @@ -725,7 +708,7 @@ describe.each([
title: 'second',
content: 'first',
});
await act(() => fetches[0]);
await act(() => fetches[1]);
expect(result.current).toEqual(
CoolerArticle.fromJS({
...payload,
Expand All @@ -742,7 +725,7 @@ describe.each([
content: 'first',
tags: ['third'],
});
await act(() => fetches[0]);
await act(() => fetches[2]);
expect(result.current).toEqual(
CoolerArticle.fromJS({
...payload,
Expand All @@ -751,6 +734,8 @@ describe.each([
tags: ['third'],
}),
);
jest.useRealTimers();
await renderDataClient.allSettled();
});

describe('race conditions', () => {
Expand Down Expand Up @@ -1033,11 +1018,16 @@ describe.each([
});

describe('with timestamps', () => {
beforeEach(() => {
jest.useFakeTimers({ legacyFakeTimers: false });
});
afterEach(() => {
jest.useRealTimers();
});
afterEach;
it.each([VisSettingsResource, VisSettingsResourceFromMixin])(
'should handle out of order server responses (%#)',
async VisResource => {
jest.useFakeTimers({ legacyFakeTimers: false });

const initVis = {
id: 5,
visType: 'graph',
Expand Down Expand Up @@ -1166,7 +1156,6 @@ describe.each([
expect(result.current.vis).toEqual(finalObject);

fetchSpy.mockClear();
jest.useRealTimers();
await renderDataClient.allSettled();
},
);
Expand Down
7 changes: 4 additions & 3 deletions website/src/components/Playground/editor-types/react.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ declare namespace React {
type JSXElementConstructor<P> =
| ((
props: P,
) => ReactNode)
) => ReactNode | Promise<ReactNode>)
// constructor signature must match React.Component
| (new(props: P) => Component<any, any>);

Expand Down Expand Up @@ -1036,7 +1036,7 @@ declare namespace React {
* ```
*/
interface FunctionComponent<P = {}> {
(props: P): ReactNode;
(props: P): ReactNode | Promise<ReactNode>;
/**
* Ignored by React.
* @deprecated Only kept in types for backwards compatibility. Will be removed in a future major release.
Expand Down Expand Up @@ -1835,10 +1835,11 @@ declare namespace React {
* A good example of this is a text input.
*
* @param value The value that is going to be deferred
* @param initialValue A value to use during the initial render of a component. If this option is omitted, `useDeferredValue` will not defer during the initial render, because there’s no previous version of `value` that it can render instead.
*
* @see {@link https://react.dev/reference/react/useDeferredValue}
*/
export function useDeferredValue<T>(value: T): T;
export function useDeferredValue<T>(value: T, initialValue?: T): T;

/**
* Allows components to avoid undesirable loading states by waiting for content to load
Expand Down

0 comments on commit 8a2c8c1

Please sign in to comment.