From 9b753d970497aeba1109d725d193b015a56b2f8f Mon Sep 17 00:00:00 2001 From: Andrey Azov Date: Tue, 27 Sep 2022 21:48:19 +0100 Subject: [PATCH 1/3] Add the runAsyncVisualTask utility --- src/shared/hooks/tests/useAsyncTask.test.ts | 159 ++++++++++++++++++++ src/shared/hooks/useAsyncTask.ts | 125 +++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 src/shared/hooks/tests/useAsyncTask.test.ts create mode 100644 src/shared/hooks/useAsyncTask.ts diff --git a/src/shared/hooks/tests/useAsyncTask.test.ts b/src/shared/hooks/tests/useAsyncTask.test.ts new file mode 100644 index 0000000000..74c9d4b426 --- /dev/null +++ b/src/shared/hooks/tests/useAsyncTask.test.ts @@ -0,0 +1,159 @@ +/** + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { timer, map, mergeMap, throwError } from 'rxjs'; + +import { runAsyncVisualTask } from '../useAsyncTask'; + +jest.useFakeTimers(); + +// The tests below are using an observable for a mock task, +// because observables, as opposed to promises, run synchronously when their timers are mocked + +describe('runAsyncVisualTask', () => { + const ignoreInterval = 100; + const minimumRunningTime = 500; + const result = 42; // obviously + + it('allows a quick task to finish without registering the loading state', () => { + const taskDuration = 90; // less than ignoreInterval + const testTask = timer(taskDuration).pipe(map(() => result)); + + const subscriber = jest.fn(); + const onComplete = jest.fn(); + + runAsyncVisualTask({ + task: testTask, + ignoreTime: ignoreInterval, + minimumRunningTime + }).subscribe({ + next: subscriber, + complete: onComplete + }); + + jest.advanceTimersByTime(1000); + + expect(subscriber).toHaveBeenCalledTimes(1); // only one value comes out of the observable + expect(subscriber.mock.calls.at(-1)[0]).toEqual({ + status: 'success', + result + }); + expect(onComplete).toHaveBeenCalled(); // just making sure that the stream completes after the task is finished + }); + + it('maintains the loading state if task completes too soon', () => { + const taskDuration = 110; // a bit more than ignoreInterval, but less than ignoreInterval + minimumRunningTime + const testTask = timer(taskDuration).pipe(map(() => result)); + + const subscriber = jest.fn(); + + runAsyncVisualTask({ + task: testTask, + ignoreTime: ignoreInterval, + minimumRunningTime + }).subscribe(subscriber); + + jest.advanceTimersByTime(taskDuration); + + expect(subscriber).toHaveBeenCalledTimes(1); + expect(subscriber.mock.calls.at(-1)[0]).toEqual({ + status: 'loading', + result: null + }); + + jest.advanceTimersByTime(minimumRunningTime); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(subscriber.mock.calls.at(-1)[0]).toEqual({ + status: 'success', + result + }); + }); + + it('runs a slow task to completion', () => { + const taskDuration = 2000; // more than ignoreInterval + minimumRunningTime + const testTask = timer(taskDuration).pipe(map(() => result)); + + const subscriber = jest.fn(); + + runAsyncVisualTask({ + task: testTask, + ignoreTime: ignoreInterval, + minimumRunningTime + }).subscribe(subscriber); + + jest.advanceTimersByTime(taskDuration - 10); + + expect(subscriber).toHaveBeenCalledTimes(1); + expect(subscriber.mock.calls.at(-1)[0]).toEqual({ + status: 'loading', + result: null + }); + + jest.runAllTimers(); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(subscriber.mock.calls.at(-1)[0]).toEqual({ + status: 'success', + result + }); + }); + + it('calls onSuccess callback if the task succeeds', () => { + const taskDuration = 2000; + const testTask = timer(taskDuration).pipe(map(() => result)); + + const successHandler = jest.fn(); + const errorHandler = jest.fn(); + + runAsyncVisualTask({ + task: testTask, + ignoreTime: ignoreInterval, + minimumRunningTime, + onComplete: successHandler, + onError: errorHandler + }).subscribe(jest.fn()); + + jest.runAllTimers(); + + expect(successHandler.mock.calls.at(-1)[0]).toEqual(result); + expect(errorHandler).not.toHaveBeenCalled(); + }); + + it('calls onError callback if the task fails', () => { + const taskDuration = 2000; + const error = new Error('oops'); + const testTask = timer(taskDuration).pipe( + mergeMap(() => throwError(error)) + ); + + const successHandler = jest.fn(); + const errorHandler = jest.fn(); + + runAsyncVisualTask({ + task: testTask, + ignoreTime: ignoreInterval, + minimumRunningTime, + onComplete: successHandler, + onError: errorHandler + }).subscribe(jest.fn()); + + jest.runAllTimers(); + + expect(errorHandler.mock.calls.at(-1)[0]).toEqual(error); + expect(successHandler).not.toHaveBeenCalled(); + }); +}); diff --git a/src/shared/hooks/useAsyncTask.ts b/src/shared/hooks/useAsyncTask.ts new file mode 100644 index 0000000000..2284b74238 --- /dev/null +++ b/src/shared/hooks/useAsyncTask.ts @@ -0,0 +1,125 @@ +/** + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + of, + from, + race, + tap, + timer, + map, + mergeMap, + combineLatest, + startWith, + catchError, + shareReplay, + distinctUntilChanged, + type Observable +} from 'rxjs'; + +// TODO: write the actual hook +const useAsyncTask = () => { + return { + run: runAsyncVisualTask + }; +}; + +/** + * This is a function for handling asynchronous tasks that are accompanied + * by a change in the UI (e.g. a spinner) indicating that they are running. + * + * It is impossible to predict how long such a task may take — it may finish + * almost immediately, or may take a long time. The purpose of the runAsyncVisualTask + * function is to avoid unwelcome flickering on the screen caused by the loading indicator + * quickly appearing and disappearing when the task is too quick to finish. + * + * Therefore, runAsyncVisualTask does the following: + * - if the task finishes very quickly (faster than the interval set by the ignoreTime option), + * then do not bother displaying the loading indicator + * - if the task takes longer than the ignoreTime period to complete, register + * the loading state, and maintain it for at least the interval defined by the minimumRunningTime + * option + * - if the task finishes after the ignoreTime interval, but before the minimumRunningTime interval, + * keep the loading state until the minimumRunningTime interval expires + * + * The implementation was inspired by https://stackblitz.com/edit/rxjs-spinner-flickering?file=index.ts + */ + +export const runAsyncVisualTask = (params: { + task: Promise | Observable; // using observable option for testability + ignoreTime?: number; + minimumRunningTime?: number; + onComplete?: (arg: T) => unknown; + onError?: (arg: unknown) => unknown; +}) => { + const { ignoreTime = 100, minimumRunningTime = 1000 } = params; + + const promise$ = from(params.task).pipe( + shareReplay(1), // make sure that task doesn't need to run from the start when it connects to withRegisteredTask + tap((result) => { + params.onComplete?.(result); + }), + map((result) => ({ + status: 'success', + result + })), + catchError((error) => { + params.onError?.(error); + + return of({ + status: 'error', + error + }); + }) + ); + + const ignoreTimer$ = timer(ignoreTime).pipe(map(() => 'registered')); + const minimumRunningTimer$ = timer(minimumRunningTime).pipe(startWith(null)); + + const withRegisteredTask = (task: typeof promise$) => { + return combineLatest([ + minimumRunningTimer$, + task.pipe(startWith({ status: 'loading', result: null })) + ]).pipe( + map(([time, result]) => { + if (time === null) { + return { status: 'loading', result: null }; + } else { + return result; + } + }), + distinctUntilChanged((prev, curr) => { + return prev.status === curr.status; + }) + ); + }; + + return race(promise$, ignoreTimer$).pipe( + mergeMap((winner) => { + if (typeof winner === 'string') { + // Meaning that the ignoreTimer$ stream has completed, + // and that we should register the task + return withRegisteredTask(promise$); + } else { + // The task completed before the ignoreTimer$ stream + // No need to report it to the user + return of(winner); + } + }) + ); +}; + +export default useAsyncTask; From a35319e8ecf635614b049eb593cdddd666fa2a9d Mon Sep 17 00:00:00 2001 From: Andrey Azov Date: Tue, 27 Sep 2022 22:20:58 +0100 Subject: [PATCH 2/3] Move runAsyncVisualTask into its own utility file --- src/shared/hooks/useAsyncVisualTask.ts | 24 +++++++++++++ .../runAsyncVisualTask.ts} | 36 ++++++++----------- .../tests/runAsyncVisualTask.test.ts} | 2 +- 3 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 src/shared/hooks/useAsyncVisualTask.ts rename src/shared/{hooks/useAsyncTask.ts => utils/runAsyncVisualTask.ts} (73%) rename src/shared/{hooks/tests/useAsyncTask.test.ts => utils/tests/runAsyncVisualTask.test.ts} (98%) diff --git a/src/shared/hooks/useAsyncVisualTask.ts b/src/shared/hooks/useAsyncVisualTask.ts new file mode 100644 index 0000000000..ef44a505fe --- /dev/null +++ b/src/shared/hooks/useAsyncVisualTask.ts @@ -0,0 +1,24 @@ +/** + * See the NOTICE file distributed with this work for additional information + * regarding copyright ownership. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// TODO: import the runAsyncVisualTask utility, and use it in the hook +const useAsyncVisualTask = () => { + return { + // run: runAsyncVisualTask + }; +}; + +export default useAsyncVisualTask; diff --git a/src/shared/hooks/useAsyncTask.ts b/src/shared/utils/runAsyncVisualTask.ts similarity index 73% rename from src/shared/hooks/useAsyncTask.ts rename to src/shared/utils/runAsyncVisualTask.ts index 2284b74238..4690721a0f 100644 --- a/src/shared/hooks/useAsyncTask.ts +++ b/src/shared/utils/runAsyncVisualTask.ts @@ -30,35 +30,29 @@ import { type Observable } from 'rxjs'; -// TODO: write the actual hook -const useAsyncTask = () => { - return { - run: runAsyncVisualTask - }; -}; - /** * This is a function for handling asynchronous tasks that are accompanied - * by a change in the UI (e.g. a spinner) indicating that they are running. + * by a change in the UI indicating that they are running (e.g. a spinner). * - * It is impossible to predict how long such a task may take — it may finish - * almost immediately, or may take a long time. The purpose of the runAsyncVisualTask - * function is to avoid unwelcome flickering on the screen caused by the loading indicator - * quickly appearing and disappearing when the task is too quick to finish. + * It is impossible to predict how long a task may take — it may finish almost immediately, + * or may run for a long time. The purpose of the runAsyncVisualTask utility function + * is to prevent unwelcome flickering on the screen caused by the loading indicator + * briefly appearing and disappearing when the task finishes too quickly + * after the indicator is displayed. * - * Therefore, runAsyncVisualTask does the following: + * The runAsyncVisualTask function wraps an asynchronous task and does the following: * - if the task finishes very quickly (faster than the interval set by the ignoreTime option), - * then do not bother displaying the loading indicator - * - if the task takes longer than the ignoreTime period to complete, register - * the loading state, and maintain it for at least the interval defined by the minimumRunningTime - * option - * - if the task finishes after the ignoreTime interval, but before the minimumRunningTime interval, - * keep the loading state until the minimumRunningTime interval expires + * then no signal for showing the loading indicator will be sent + * - if the task takes longer than the ignoreTime period to complete, then the function + * sends a signal to show the loading indicator, and waits for at least the length of time + * defined by the minimumRunningTime option before sending a different signal + * - thus, if the task completes after the ignoreTime interval, but before the minimumRunningTime expires, + * then its completion will be reported only after the minimumRunningTime interval runs out * * The implementation was inspired by https://stackblitz.com/edit/rxjs-spinner-flickering?file=index.ts */ -export const runAsyncVisualTask = (params: { +const runAsyncVisualTask = (params: { task: Promise | Observable; // using observable option for testability ignoreTime?: number; minimumRunningTime?: number; @@ -122,4 +116,4 @@ export const runAsyncVisualTask = (params: { ); }; -export default useAsyncTask; +export default runAsyncVisualTask; diff --git a/src/shared/hooks/tests/useAsyncTask.test.ts b/src/shared/utils/tests/runAsyncVisualTask.test.ts similarity index 98% rename from src/shared/hooks/tests/useAsyncTask.test.ts rename to src/shared/utils/tests/runAsyncVisualTask.test.ts index 74c9d4b426..dfbe44f6bf 100644 --- a/src/shared/hooks/tests/useAsyncTask.test.ts +++ b/src/shared/utils/tests/runAsyncVisualTask.test.ts @@ -16,7 +16,7 @@ import { timer, map, mergeMap, throwError } from 'rxjs'; -import { runAsyncVisualTask } from '../useAsyncTask'; +import runAsyncVisualTask from '../runAsyncVisualTask'; jest.useFakeTimers(); From 9ac865eb5e3f827ce6fb84a6d5287dac0bac434e Mon Sep 17 00:00:00 2001 From: Andrey Azov Date: Tue, 27 Sep 2022 22:31:28 +0100 Subject: [PATCH 3/3] Rename promise$ to task$ --- src/shared/utils/runAsyncVisualTask.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/utils/runAsyncVisualTask.ts b/src/shared/utils/runAsyncVisualTask.ts index 4690721a0f..9dc6cceafd 100644 --- a/src/shared/utils/runAsyncVisualTask.ts +++ b/src/shared/utils/runAsyncVisualTask.ts @@ -61,8 +61,8 @@ const runAsyncVisualTask = (params: { }) => { const { ignoreTime = 100, minimumRunningTime = 1000 } = params; - const promise$ = from(params.task).pipe( - shareReplay(1), // make sure that task doesn't need to run from the start when it connects to withRegisteredTask + const task$ = from(params.task).pipe( + shareReplay(1), // make sure that the task won't need to run from the start when it is consumed by withRegisteredTask tap((result) => { params.onComplete?.(result); }), @@ -83,7 +83,7 @@ const runAsyncVisualTask = (params: { const ignoreTimer$ = timer(ignoreTime).pipe(map(() => 'registered')); const minimumRunningTimer$ = timer(minimumRunningTime).pipe(startWith(null)); - const withRegisteredTask = (task: typeof promise$) => { + const withRegisteredTask = (task: typeof task$) => { return combineLatest([ minimumRunningTimer$, task.pipe(startWith({ status: 'loading', result: null })) @@ -101,12 +101,12 @@ const runAsyncVisualTask = (params: { ); }; - return race(promise$, ignoreTimer$).pipe( + return race(task$, ignoreTimer$).pipe( mergeMap((winner) => { if (typeof winner === 'string') { // Meaning that the ignoreTimer$ stream has completed, // and that we should register the task - return withRegisteredTask(promise$); + return withRegisteredTask(task$); } else { // The task completed before the ignoreTimer$ stream // No need to report it to the user