react-idle-timer: throttle doesn't work at all

First, love this package. There is just one small problem I am experiencing, and I am not sure why.

code:

const { getRemainingTime, getLastActiveTime } = useIdleTimer({
    timeout: 1000 * 20,// after 20s of activity, user is idle. next time user becomes active, will immediately call server
    onIdle: handleOnIdle,
    onActive: handleOnActive,
    onAction: handleOnAction,
    debounce: 0,
    throttle: 60000, // will only call server at most every 60 seconds for onAction
    eventsThrottle: 500, // prevent too much cpu usage for user actions
    capture: false,
    passive: true
  });

expected:

onAction fires at most once every 60 seconds

actual:

onAction fires every ~500ms.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 33 (21 by maintainers)

Most upvoted comments

When 4.6.1 drops you will need to wrap your handlers in useCallback. Just a heads up.

SupremeTechnopriest on Apr 12, 2021

Confirmed. Will look into this tonight.

SupremeTechnopriest on Apr 5, 2021

Hey @roypearce and @r3wt,

I made some discoveries while working on some new features in regards to throttle. There is a trade off we need to decide on. So basically if the component that mounts IdleTimer is functional, the callbacks onActive, onIdle and onAction are recreated every time the component updates. This is why we were seeing that bug with the throttle. The reason being is that we have the useIdleTimer hook set up so that you can redefine the event handlers. We need to decide between being able to redefine handlers and wrapping them in useCallback or not being able to redefine them and being able to pass regular functions.

const handlOnActive  = useCallback(() => console.log('active'), [])
idleTimer({ onActive: handleOnActive })

// vs

idleTimer({ onActive: () => console.log('active') })

I can make either implementation work, but if we want to keep the ability to dynamically reassign the event handlers, you would have to wrap them in useCallback at the application level. What do you think?

SupremeTechnopriest on Apr 12, 2021

@SupremeTechnopriest thank you sir!

roypearce on Apr 9, 2021

If you guys need this on the stable branch I can push a patch, just let me know if you cant wait a week and I’ll get it done.

SupremeTechnopriest on Apr 6, 2021

@r3wt Interesting! I will take a look at this and add some tests.

SupremeTechnopriest on Aug 12, 2020