From 087039f0ccd13e9fe5bf4ef904e4f1c2df129d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ali=20Emir=20=C5=9Een?= Date: Tue, 7 Jan 2025 10:34:11 +0300 Subject: [PATCH] refactor(core): remove duplicated overtime intervals (#6626) --- .changeset/giant-carrots-deny.md | 9 ++++ .changeset/perfect-donkeys-cheat.md | 11 ++++ .../docs/core/refine-component/index.md | 5 ++ packages/core/src/contexts/refine/index.tsx | 1 + .../helpers/handleRefineOptions/index.spec.ts | 3 ++ packages/core/src/hooks/data/useCreate.ts | 3 +- packages/core/src/hooks/data/useCreateMany.ts | 3 +- packages/core/src/hooks/data/useCustom.ts | 3 +- .../core/src/hooks/data/useCustomMutation.ts | 3 +- packages/core/src/hooks/data/useDelete.ts | 3 +- packages/core/src/hooks/data/useDeleteMany.ts | 3 +- .../core/src/hooks/data/useInfiniteList.ts | 3 +- packages/core/src/hooks/data/useList.ts | 3 +- packages/core/src/hooks/data/useMany.ts | 3 +- packages/core/src/hooks/data/useOne.ts | 3 +- packages/core/src/hooks/data/useUpdate.ts | 3 +- packages/core/src/hooks/data/useUpdateMany.ts | 3 +- packages/core/src/hooks/form/index.ts | 6 ++- packages/core/src/hooks/show/index.ts | 11 ++-- .../hooks/useLoadingOvertime/index.spec.tsx | 21 ++++++++ .../src/hooks/useLoadingOvertime/index.ts | 22 ++++++-- packages/core/src/hooks/useSelect/index.ts | 5 +- .../core/src/hooks/useTable/index.spec.ts | 53 +++++++++++++++++++ packages/core/src/hooks/useTable/index.ts | 18 ++----- 24 files changed, 149 insertions(+), 52 deletions(-) create mode 100644 .changeset/giant-carrots-deny.md create mode 100644 .changeset/perfect-donkeys-cheat.md diff --git a/.changeset/giant-carrots-deny.md b/.changeset/giant-carrots-deny.md new file mode 100644 index 000000000000..27c7b2a6b688 --- /dev/null +++ b/.changeset/giant-carrots-deny.md @@ -0,0 +1,9 @@ +--- +"@refinedev/core": patch +--- + +feat(core): add `enabled` prop to `useLoadingOvertime` and `overtimeOptions` + +Added missing `enabled` prop to `useLoadingOvertime` and added ability to globally configure through `options.overtime.enabled`. + +Due to the nature of calculating elapsed time, an interval is set by the `interval` prop. This was causing unwanted updates in the return value and there was no way to disable it properly. diff --git a/.changeset/perfect-donkeys-cheat.md b/.changeset/perfect-donkeys-cheat.md new file mode 100644 index 000000000000..f18a206ad6b5 --- /dev/null +++ b/.changeset/perfect-donkeys-cheat.md @@ -0,0 +1,11 @@ +--- +"@refinedev/core": patch +--- + +refactor(core): remove duplicated overtime intervals caused by internally used hooks + +Updated Refine's data hooks and extensions to prevent duplicated overtime intervals from being created. This uses the `enabled` prop to prevent internal hooks from registering the intervals. + +Prior to this change, `useTable` was initializing its own `useLoadingOvertime` hook but also propagated the `elapsedTime` from `useList` hook which is used internally by `useTable`. This caused duplicated intervals and unwanted updates. + +This now ensures a single interval is created and used for the extension hooks. diff --git a/documentation/docs/core/refine-component/index.md b/documentation/docs/core/refine-component/index.md index 0ce6de0a8cea..08c89aa813c0 100644 --- a/documentation/docs/core/refine-component/index.md +++ b/documentation/docs/core/refine-component/index.md @@ -604,6 +604,7 @@ const App = () => ( // highlight-start options={{ overtime: { + enabled: true, interval: 1000, // default value is 1000 onInterval: (elapsedInterval, context) => { console.log(elapsedInterval, context); @@ -615,6 +616,10 @@ const App = () => ( ); ``` +#### enabled + +If true, the elapsed time will be calculated. If set to false, the elapsed time will always be `undefined`. + #### interval The interval value in milliseconds. The default value is `1000`. diff --git a/packages/core/src/contexts/refine/index.tsx b/packages/core/src/contexts/refine/index.tsx index 33b338575799..c91a1003da83 100644 --- a/packages/core/src/contexts/refine/index.tsx +++ b/packages/core/src/contexts/refine/index.tsx @@ -51,6 +51,7 @@ export const defaultRefineOptions: IRefineContextOptions = { afterEdit: "list", }, overtime: { + enabled: true, interval: 1000, }, textTransformers: { diff --git a/packages/core/src/definitions/helpers/handleRefineOptions/index.spec.ts b/packages/core/src/definitions/helpers/handleRefineOptions/index.spec.ts index 162a5762500d..e1f4e94f01ac 100644 --- a/packages/core/src/definitions/helpers/handleRefineOptions/index.spec.ts +++ b/packages/core/src/definitions/helpers/handleRefineOptions/index.spec.ts @@ -53,6 +53,7 @@ describe("handleRefineOptions", () => { }, breadcrumb: false, overtime: { + enabled: true, interval: 1000, }, textTransformers: { @@ -123,6 +124,7 @@ describe("handleRefineOptions", () => { afterEdit: "list", }, overtime: { + enabled: true, interval: 1000, }, textTransformers: { @@ -177,6 +179,7 @@ describe("handleRefineOptions", () => { afterEdit: "list", }, overtime: { + enabled: true, interval: 1000, }, textTransformers: { diff --git a/packages/core/src/hooks/data/useCreate.ts b/packages/core/src/hooks/data/useCreate.ts index eb1aa0938970..de0f1c0d90fe 100644 --- a/packages/core/src/hooks/data/useCreate.ts +++ b/packages/core/src/hooks/data/useCreate.ts @@ -315,9 +315,8 @@ export const useCreate = < const { mutate, mutateAsync, ...mutation } = mutationResult; const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); // this function is used to make the `variables` parameter optional diff --git a/packages/core/src/hooks/data/useCreateMany.ts b/packages/core/src/hooks/data/useCreateMany.ts index 4a8ccc859bf3..d6fb6a122ece 100644 --- a/packages/core/src/hooks/data/useCreateMany.ts +++ b/packages/core/src/hooks/data/useCreateMany.ts @@ -293,9 +293,8 @@ export const useCreateMany = < const { mutate, mutateAsync, ...mutation } = mutationResult; const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); // this function is used to make the `variables` parameter optional diff --git a/packages/core/src/hooks/data/useCustom.ts b/packages/core/src/hooks/data/useCustom.ts index 41bf2c0ed4a8..85c8d24b72f8 100644 --- a/packages/core/src/hooks/data/useCustom.ts +++ b/packages/core/src/hooks/data/useCustom.ts @@ -217,9 +217,8 @@ export const useCustom = < }, }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResponse.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...queryResponse, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useCustomMutation.ts b/packages/core/src/hooks/data/useCustomMutation.ts index a39c89e34e7e..5f419348af0d 100644 --- a/packages/core/src/hooks/data/useCustomMutation.ts +++ b/packages/core/src/hooks/data/useCustomMutation.ts @@ -215,9 +215,8 @@ export const useCustomMutation = < ); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...mutation, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useDelete.ts b/packages/core/src/hooks/data/useDelete.ts index 2cecc9f8a1b4..a3b812524531 100644 --- a/packages/core/src/hooks/data/useDelete.ts +++ b/packages/core/src/hooks/data/useDelete.ts @@ -493,9 +493,8 @@ export const useDelete = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...mutation, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useDeleteMany.ts b/packages/core/src/hooks/data/useDeleteMany.ts index 49152d83ad41..1412a2e53834 100644 --- a/packages/core/src/hooks/data/useDeleteMany.ts +++ b/packages/core/src/hooks/data/useDeleteMany.ts @@ -524,9 +524,8 @@ export const useDeleteMany = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...mutation, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useInfiniteList.ts b/packages/core/src/hooks/data/useInfiniteList.ts index f7fc38b04fc9..d1769de4ff18 100644 --- a/packages/core/src/hooks/data/useInfiniteList.ts +++ b/packages/core/src/hooks/data/useInfiniteList.ts @@ -320,9 +320,8 @@ export const useInfiniteList = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResponse.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...queryResponse, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useList.ts b/packages/core/src/hooks/data/useList.ts index 0984d4775078..aa6f7dcb8d55 100644 --- a/packages/core/src/hooks/data/useList.ts +++ b/packages/core/src/hooks/data/useList.ts @@ -326,9 +326,8 @@ export const useList = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResponse.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...queryResponse, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useMany.ts b/packages/core/src/hooks/data/useMany.ts index ac69ad8ec975..992751234cb4 100644 --- a/packages/core/src/hooks/data/useMany.ts +++ b/packages/core/src/hooks/data/useMany.ts @@ -240,9 +240,8 @@ export const useMany = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResponse.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...queryResponse, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useOne.ts b/packages/core/src/hooks/data/useOne.ts index fa47d6380f76..94ccdaa1d9de 100644 --- a/packages/core/src/hooks/data/useOne.ts +++ b/packages/core/src/hooks/data/useOne.ts @@ -244,9 +244,8 @@ export const useOne = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResponse.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); return { ...queryResponse, overtime: { elapsedTime } }; diff --git a/packages/core/src/hooks/data/useUpdate.ts b/packages/core/src/hooks/data/useUpdate.ts index e139fd0c93e8..d88c3d409988 100644 --- a/packages/core/src/hooks/data/useUpdate.ts +++ b/packages/core/src/hooks/data/useUpdate.ts @@ -639,9 +639,8 @@ export const useUpdate = < const { mutate, mutateAsync, ...mutation } = mutationResult; const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); // this function is used to make the `variables` parameter optional diff --git a/packages/core/src/hooks/data/useUpdateMany.ts b/packages/core/src/hooks/data/useUpdateMany.ts index ced9e0dae6ca..5eef842e7b0a 100644 --- a/packages/core/src/hooks/data/useUpdateMany.ts +++ b/packages/core/src/hooks/data/useUpdateMany.ts @@ -676,9 +676,8 @@ export const useUpdateMany = < const { mutate, mutateAsync, ...mutation } = mutationResult; const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: mutation.isLoading, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); // this function is used to make the `variables` parameter optional diff --git a/packages/core/src/hooks/form/index.ts b/packages/core/src/hooks/form/index.ts index 9f09b3663482..feffcadf2ba2 100644 --- a/packages/core/src/hooks/form/index.ts +++ b/packages/core/src/hooks/form/index.ts @@ -169,14 +169,17 @@ export const useForm = < liveParams: props.liveParams, meta: { ...combinedMeta, ...props.queryMeta }, dataProviderName: props.dataProviderName, + overtimeOptions: { enabled: false }, }); const createMutation = useCreate({ mutationOptions: props.createMutationOptions, + overtimeOptions: { enabled: false }, }); const updateMutation = useUpdate({ mutationOptions: props.updateMutationOptions, + overtimeOptions: { enabled: false }, }); const mutationResult = isEdit ? updateMutation : createMutation; @@ -184,9 +187,8 @@ export const useForm = < const formLoading = isMutationLoading || queryResult.isFetching; const { elapsedTime } = useLoadingOvertime({ + ...props.overtimeOptions, isLoading: formLoading, - interval: props.overtimeOptions?.interval, - onInterval: props.overtimeOptions?.onInterval, }); React.useEffect(() => { diff --git a/packages/core/src/hooks/show/index.ts b/packages/core/src/hooks/show/index.ts index f26c9942a746..c7721b4311bd 100644 --- a/packages/core/src/hooks/show/index.ts +++ b/packages/core/src/hooks/show/index.ts @@ -1,5 +1,5 @@ import warnOnce from "warn-once"; -import { useMeta, useOne, useResourceParams, useLoadingOvertime } from "@hooks"; +import { useMeta, useOne, useResourceParams } from "@hooks"; import { pickNotDeprecated } from "@definitions/helpers"; import type { UseShowProps, UseShowReturnType } from "./types"; @@ -71,21 +71,16 @@ export const useShow = < }, meta: combinedMeta, metaData: combinedMeta, + overtimeOptions, ...useOneProps, }); - const { elapsedTime } = useLoadingOvertime({ - isLoading: queryResult.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, - }); - return { queryResult, query: queryResult, showId, setShowId, - overtime: { elapsedTime }, + overtime: queryResult.overtime, }; }; diff --git a/packages/core/src/hooks/useLoadingOvertime/index.spec.tsx b/packages/core/src/hooks/useLoadingOvertime/index.spec.tsx index c686440a1c12..6adad9dc022b 100644 --- a/packages/core/src/hooks/useLoadingOvertime/index.spec.tsx +++ b/packages/core/src/hooks/useLoadingOvertime/index.spec.tsx @@ -147,4 +147,25 @@ describe("useLoadingOvertime Hook", () => { expect(onInterval).toBeCalledTimes(1); expect(onInterval).toBeCalledWith(1000); }); + + it("should not run interval when enabled is false", () => { + const { result } = renderHook( + () => + useLoadingOvertime({ + isLoading: true, + enabled: false, + }), + { + wrapper: TestWrapper({}), + }, + ); + + act(() => { + jest.advanceTimersByTime(2000); + }); + + const { elapsedTime } = result.current; + + expect(elapsedTime).toBeUndefined(); + }); }); diff --git a/packages/core/src/hooks/useLoadingOvertime/index.ts b/packages/core/src/hooks/useLoadingOvertime/index.ts index 2884f621d43b..4ed483ccd151 100644 --- a/packages/core/src/hooks/useLoadingOvertime/index.ts +++ b/packages/core/src/hooks/useLoadingOvertime/index.ts @@ -27,6 +27,13 @@ type UseLoadingOvertimeCoreReturnType = { }; export type UseLoadingOvertimeCoreProps = { + /** + * If true, the elapsed time will be calculated. If set to false; the elapsed time will be `undefined`. + * + * @default: true + */ + enabled?: boolean; + /** * The loading state. If true, the elapsed time will be calculated. */ @@ -63,6 +70,7 @@ export type UseLoadingOvertimeCoreProps = { * }); */ export const useLoadingOvertime = ({ + enabled: enabledProp, isLoading, interval: intervalProp, onInterval: onIntervalProp, @@ -75,11 +83,17 @@ export const useLoadingOvertime = ({ // pick props or refine context options const interval = intervalProp ?? overtime.interval; const onInterval = onIntervalProp ?? overtime?.onInterval; + const enabled = + typeof enabledProp !== "undefined" + ? enabledProp + : typeof overtime.enabled !== "undefined" + ? overtime.enabled + : true; useEffect(() => { let intervalFn: ReturnType; - if (isLoading) { + if (enabled && isLoading) { intervalFn = setInterval(() => { // increase elapsed time setElapsedTime((prevElapsedTime) => { @@ -93,11 +107,13 @@ export const useLoadingOvertime = ({ } return () => { - clearInterval(intervalFn); + if (typeof intervalFn !== "undefined") { + clearInterval(intervalFn); + } // reset elapsed time setElapsedTime(undefined); }; - }, [isLoading, interval]); + }, [isLoading, interval, enabled]); useEffect(() => { // call onInterval callback diff --git a/packages/core/src/hooks/useSelect/index.ts b/packages/core/src/hooks/useSelect/index.ts index 288042e8c44d..01f500fb86ac 100644 --- a/packages/core/src/hooks/useSelect/index.ts +++ b/packages/core/src/hooks/useSelect/index.ts @@ -303,6 +303,7 @@ export const useSelect = < defaultValueQueryOptions?.onSuccess?.(data); }, }, + overtimeOptions: { enabled: false }, meta: combinedMeta, metaData: combinedMeta, liveMode: "off", @@ -341,6 +342,7 @@ export const useSelect = < queryOptions?.onSuccess?.(data); }, }, + overtimeOptions: { enabled: false }, successNotification, errorNotification, meta: combinedMeta, @@ -352,9 +354,8 @@ export const useSelect = < }); const { elapsedTime } = useLoadingOvertime({ + ...overtimeOptions, isLoading: queryResult.isFetching || defaultValueQueryResult.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, }); const combinedOptions = useMemo( diff --git a/packages/core/src/hooks/useTable/index.spec.ts b/packages/core/src/hooks/useTable/index.spec.ts index c90fdc27eeb7..1f46cf3cd419 100644 --- a/packages/core/src/hooks/useTable/index.spec.ts +++ b/packages/core/src/hooks/useTable/index.spec.ts @@ -15,6 +15,7 @@ import type { Pagination, } from "../../contexts/data/types"; import * as useRouterType from "../../contexts/router/picker"; +import { defaultRefineOptions } from "@contexts/refine"; const defaultPagination = { pageSize: 10, @@ -473,6 +474,58 @@ describe("useTable Hook", () => { }); }); + it("should call onInterval once at ticks (no duplicates)", async () => { + const onInterval = jest.fn(); + const { result } = renderHook( + () => + useTable({ + resource: "posts", + }), + { + wrapper: TestWrapper({ + refineProvider: { + hasDashboard: false, + mutationMode: "pessimistic", + syncWithLocation: false, + warnWhenUnsavedChanges: false, + undoableTimeout: 5000, + liveMode: "off", + options: { + ...defaultRefineOptions, + overtime: { + enabled: true, + interval: 100, + onInterval, + }, + }, + }, + dataProvider: { + default: { + ...MockJSONServer.default, + getList: () => { + return new Promise((res) => { + setTimeout(() => res({ data: [], total: 2 }), 1000); + }); + }, + }, + }, + resources: [{ name: "posts" }], + }), + }, + ); + + await waitFor(() => { + expect(result.current.tableQueryResult.isFetching).toBeTruthy(); + expect(result.current.overtime.elapsedTime).toBe(900); + expect(onInterval).toBeCalledTimes(9); + }); + + await waitFor(() => { + expect(!result.current.tableQueryResult.isFetching).toBeTruthy(); + expect(result.current.overtime.elapsedTime).toBeUndefined(); + }); + }); + it("should work with tableQuery and tableQueryResult", async () => { const { result } = renderHook(() => useTable(), { wrapper: TestWrapper({ diff --git a/packages/core/src/hooks/useTable/index.ts b/packages/core/src/hooks/useTable/index.ts index b679e5392468..9a31670ac1e8 100644 --- a/packages/core/src/hooks/useTable/index.ts +++ b/packages/core/src/hooks/useTable/index.ts @@ -44,10 +44,9 @@ import type { import type { LiveModeProps } from "../../contexts/live/types"; import type { SuccessErrorNotification } from "../../contexts/notification/types"; import type { BaseListProps } from "../data/useList"; -import { - type UseLoadingOvertimeOptionsProps, - type UseLoadingOvertimeReturnType, - useLoadingOvertime, +import type { + UseLoadingOvertimeOptionsProps, + UseLoadingOvertimeReturnType, } from "../useLoadingOvertime"; type SetFilterBehavior = "merge" | "replace"; @@ -510,6 +509,7 @@ export function useTable< ? unionSorters(preferredPermanentSorters, sorters) : undefined, queryOptions, + overtimeOptions, successNotification, errorNotification, meta: combinedMeta, @@ -571,12 +571,6 @@ export function useTable< [preferredPermanentSorters], ); - const { elapsedTime } = useLoadingOvertime({ - isLoading: queryResult.isFetching, - interval: overtimeOptions?.interval, - onInterval: overtimeOptions?.onInterval, - }); - return { tableQueryResult: queryResult, tableQuery: queryResult, @@ -594,8 +588,6 @@ export function useTable< ? Math.ceil((queryResult.data?.total ?? 0) / pageSize) : 1, createLinkForSyncWithLocation, - overtime: { - elapsedTime, - }, + overtime: queryResult.overtime, }; }