Skip to content

Commit

Permalink
refactor(core): remove duplicated overtime intervals (#6626)
Browse files Browse the repository at this point in the history
  • Loading branch information
aliemir authored Jan 7, 2025
1 parent 4852c80 commit 087039f
Show file tree
Hide file tree
Showing 24 changed files with 149 additions and 52 deletions.
9 changes: 9 additions & 0 deletions .changeset/giant-carrots-deny.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions .changeset/perfect-donkeys-cheat.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions documentation/docs/core/refine-component/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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`.
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/contexts/refine/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const defaultRefineOptions: IRefineContextOptions = {
afterEdit: "list",
},
overtime: {
enabled: true,
interval: 1000,
},
textTransformers: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe("handleRefineOptions", () => {
},
breadcrumb: false,
overtime: {
enabled: true,
interval: 1000,
},
textTransformers: {
Expand Down Expand Up @@ -123,6 +124,7 @@ describe("handleRefineOptions", () => {
afterEdit: "list",
},
overtime: {
enabled: true,
interval: 1000,
},
textTransformers: {
Expand Down Expand Up @@ -177,6 +179,7 @@ describe("handleRefineOptions", () => {
afterEdit: "list",
},
overtime: {
enabled: true,
interval: 1000,
},
textTransformers: {
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useCreate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useCreateMany.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useCustom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,8 @@ export const useCustom = <
},
});
const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: queryResponse.isFetching,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...queryResponse, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useCustomMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,8 @@ export const useCustomMutation = <
);

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: mutation.isLoading,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...mutation, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useDelete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,9 +493,8 @@ export const useDelete = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: mutation.isLoading,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...mutation, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useDeleteMany.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,9 +524,8 @@ export const useDeleteMany = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: mutation.isLoading,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...mutation, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useInfiniteList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,8 @@ export const useInfiniteList = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: queryResponse.isFetching,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...queryResponse, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,8 @@ export const useList = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: queryResponse.isFetching,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...queryResponse, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useMany.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,8 @@ export const useMany = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: queryResponse.isFetching,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...queryResponse, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useOne.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,8 @@ export const useOne = <
});

const { elapsedTime } = useLoadingOvertime({
...overtimeOptions,
isLoading: queryResponse.isFetching,
interval: overtimeOptions?.interval,
onInterval: overtimeOptions?.onInterval,
});

return { ...queryResponse, overtime: { elapsedTime } };
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useUpdate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/hooks/data/useUpdateMany.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/hooks/form/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,24 +169,26 @@ export const useForm = <
liveParams: props.liveParams,
meta: { ...combinedMeta, ...props.queryMeta },
dataProviderName: props.dataProviderName,
overtimeOptions: { enabled: false },
});

const createMutation = useCreate<TResponse, TResponseError, TVariables>({
mutationOptions: props.createMutationOptions,
overtimeOptions: { enabled: false },
});

const updateMutation = useUpdate<TResponse, TResponseError, TVariables>({
mutationOptions: props.updateMutationOptions,
overtimeOptions: { enabled: false },
});

const mutationResult = isEdit ? updateMutation : createMutation;
const isMutationLoading = mutationResult.isLoading;
const formLoading = isMutationLoading || queryResult.isFetching;

const { elapsedTime } = useLoadingOvertime({
...props.overtimeOptions,
isLoading: formLoading,
interval: props.overtimeOptions?.interval,
onInterval: props.overtimeOptions?.onInterval,
});

React.useEffect(() => {
Expand Down
11 changes: 3 additions & 8 deletions packages/core/src/hooks/show/index.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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,
};
};

Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/hooks/useLoadingOvertime/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
22 changes: 19 additions & 3 deletions packages/core/src/hooks/useLoadingOvertime/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -63,6 +70,7 @@ export type UseLoadingOvertimeCoreProps = {
* });
*/
export const useLoadingOvertime = ({
enabled: enabledProp,
isLoading,
interval: intervalProp,
onInterval: onIntervalProp,
Expand All @@ -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<typeof setInterval>;

if (isLoading) {
if (enabled && isLoading) {
intervalFn = setInterval(() => {
// increase elapsed time
setElapsedTime((prevElapsedTime) => {
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/hooks/useSelect/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ export const useSelect = <
defaultValueQueryOptions?.onSuccess?.(data);
},
},
overtimeOptions: { enabled: false },
meta: combinedMeta,
metaData: combinedMeta,
liveMode: "off",
Expand Down Expand Up @@ -341,6 +342,7 @@ export const useSelect = <
queryOptions?.onSuccess?.(data);
},
},
overtimeOptions: { enabled: false },
successNotification,
errorNotification,
meta: combinedMeta,
Expand All @@ -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(
Expand Down
Loading

0 comments on commit 087039f

Please sign in to comment.