reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.63k stars 1.15k forks source link

pollingInterval to support a function value #2639

Open ymulenll opened 2 years ago

ymulenll commented 2 years ago

It will be very useful for pollingInterval to allow us to pass a function with the latest result, so we can stop polling based on the result.

e.g.

  const { data: executionData, error } = useGetExecutionQuery(executionId, {
    pollingInterval: (data) =>
      executionData.status === "RUNNING"
        ? 3000 // polling every 3 seconds when the status is still running
        : 0,
  });
phryneas commented 2 years ago

Why does this need to be a function? If data changes, the component rerenders.

This already works:

  const { data: executionData, error } = useGetExecutionQuery(executionId, {
    pollingInterval: executionData.status === "RUNNING"
        ? 3000 // polling every 3 seconds when the status is still running
        : 0,
  });
ymulenll commented 2 years ago

Why does this need to be a function? If data changes, the component rerenders.

This already works:

  const { data: executionData, error } = useGetExecutionQuery(executionId, {
    pollingInterval: executionData.status === "RUNNING"
        ? 3000 // polling every 3 seconds when the status is still running
        : 0,
  });

That will give you an error:

ReferenceError
Cannot access 'executionData' before initialization

react-query has that option to pass a function, and it's useful for those cases when you want to stop polling based on the result.

refetchInterval: number | false | ((data: TData | undefined, query: Query) => number | false) Optional If set to a number, all queries will continuously refetch at this frequency in milliseconds If set to a function, the function will be executed with the latest data and query to compute a frequency

https://github.com/TanStack/query/blob/e7171444f136f6adebbc2ae04fb6dbbe04a8ead6/docs/reference/useQuery.md

phryneas commented 2 years ago

Huh, point taken. I'm not sure if making the api more complicated is the right step to solve this though. Gotta think about that.

In the meantime, something like

 const { data: executionData, error } = useGetExecutionQuery(executionId)
 useGetExecutionQuery(executionId, {
    pollingInterval: executionData.status === "RUNNING"
        ? 3000 // polling every 3 seconds when the status is still running
        : 0,
  });

should definitely work though.

ymulenll commented 2 years ago

Huh, point taken. I'm not sure if making the api more complicated is the right step to solve this though. Gotta think about that.

In the meantime, something like

 const { data: executionData, error } = useGetExecutionQuery(executionId)
 useGetExecutionQuery(executionId, {
    pollingInterval: executionData.status === "RUNNING"
        ? 3000 // polling every 3 seconds when the status is still running
        : 0,
  });

should definitely work though.

In my experience it is useful, but I am not sure how complicated it could be to implement internally, the 2 hooks workaround does not look super clean, but it works for sure 👍 , thanks.

akinnee commented 1 year ago

Currently we are using useEffect + useState to disable polling after the data has a certain property:

const [enablePolling, setEnablePolling] = useState(true);
pollingInterval: enablePolling ? 3000 : 0,
useEffect(() => {
  setEnablePolling(executionData && executionData.status === "RUNNING");
}, [executionData?.status]);

I agree it would be nice to have something like:

pollingInterval: (data) => executionData.status === "RUNNING" ? 3000 : 0,
cynthia-shu commented 1 year ago

I agree it would be nice to have something like:

pollingInterval: (data) => executionData.status === "RUNNING" ? 3000 : 0,

+1 on this