Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

throttle doesn't work if bound to props! #74

Closed
grantbowering opened this issue Mar 20, 2019 · 2 comments
Closed

throttle doesn't work if bound to props! #74

grantbowering opened this issue Mar 20, 2019 · 2 comments

Comments

@grantbowering
Copy link

Given this markup,

            <IdleTimer
                onAction={this.onExternalUserIsStillWorking}
                throttle={this.props.externalKeepAliveThrottleMs}
            />

the result is

Uncaught TypeError: n.throttledAction is not a function
    at HTMLDocument.n._handleEvent (index.es.js:66)

every time the user moves the mouse or anything else.
(in versions 4.1.3 and 4.2.3)

I haven't cloned the code, just skimmed it quickly in-browser on github, so forgive me if this makes no sense, but I think this is because throttledAction is set to a throttled props.onAction if props.throttle > 0 during the constructor, but is never updated if the component receives props after that.
Meanwhile, _handleEvent checks if throttle > 0 live from this.props and tries to call this.throttledAction(e) if it is.
But if props.throttle didn't have a value during the constructor, this.throttledAction is undefined, even though props.throttle does have a value now.
It looks like _handleEvent is the only place that this.throttledAction is used...
Maybe just call throttled(onAction, throttle)() directly from _handleEvent?
No, I guess that might get it garbage-collected if there's no actual reference to the function...
What about something like this:

    // Fire debounced, throttled or raw onAction callback with event
    if (debounce > 0) {
+     if (onAction && !this.debouncedAction) {
+       this.debouncedAction = debounced(props.onAction, props.debounce);
+     }
      this.debouncedAction(e);
    } else if (throttle > 0) {
+     if (onAction && !this.throttledAction) {
+       this.throttledAction = throttled(props.onAction, props.throttle);
+     }
      this.throttledAction(e);
    } else {
      onAction(e);
    }
@SupremeTechnopriest
Copy link
Owner

Hey @grantbowering. I believe you are correct. I will add a test for this case tonight and get it fixed for you.

@SupremeTechnopriest
Copy link
Owner

Hey @grantbowering, I implemented this slightly differently using lifecycle methods. I also added some tests and everything passes. Try out 4.2.4 from npm and let me know if there are any more issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants