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

Weird Edge-Casey Render Race #40

Closed
ericf89 opened this issue Jul 18, 2016 · 7 comments · Fixed by #47
Closed

Weird Edge-Casey Render Race #40

ericf89 opened this issue Jul 18, 2016 · 7 comments · Fixed by #47

Comments

@ericf89
Copy link
Contributor

ericf89 commented Jul 18, 2016

I feel like I'm probably running into some issues that are specific to my particular use case. I ended up forking and fixing them here, but it probably is too specific to my case to be merged in. Thought I'd let you look it over though just in case. 😄

I have a situation where my debounced input is re-rendered (by a component higher up the tree) while it's debouncing its onChange handler. This means it receives props, sets state, and renders with it's props value which is normally a couple keystrokes behind the user, before triggering it's change handler that properly updates the value of the input in the store, which trickles all the way back down again.

The result is a kinda flicker back and forth between the correct value and the one in props as everything stabilizes.

I 'fixed' it by keeping track of whether or not the debounced input is currently debouncing. If so, it ignores value changes in its props, (under the assumption that it'll change again when the debounced handler actually fires/notifies. I don't really like ignoring the props change... but it seemed like the simplest way to fix my issue. 💀

Let me know if you think this is actually useful, and I can formalize it into a PR, or you can cannibalize it how you see fit.

If not you can close this, and I'll use my version for my use case. Cheers!

@nkbt
Copy link
Owner

nkbt commented Jul 27, 2016

Thanks for sharing!
I think it is possible... Though I would rather use this.isDebouncing instead of state. And also it makes sense to check if debounce has any notion of if it is now debouncing something.

@ericf89
Copy link
Contributor Author

ericf89 commented Jul 29, 2016

Good point! Don't know what I was thinking with the state.

I looked through the lodash docs and don't see any api for determining the debounce state, but I'll investigate this a little more, clean it up, and send over a PR.

@nkbt
Copy link
Owner

nkbt commented Jul 29, 2016

Thanks, really appreciate it

@nkbt
Copy link
Owner

nkbt commented Jul 29, 2016

Nice catch with this.notify.flush();

@scandinaro
Copy link

@ericf89 I'm having this same issue. Do you have a fork I can use with your fix?

@ericf89
Copy link
Contributor Author

ericf89 commented Nov 9, 2016

Hey @scandinaro, sorry for the delay. I remember installing this commit for a while and I think it worked ok. The branch is little behind the current master though, so you may be missing other useful things though. Hope it helps!

@nkbt nkbt closed this as completed in #47 Nov 18, 2016
@nkbt nkbt removed the in progress label Nov 18, 2016
@nkbt
Copy link
Owner

nkbt commented Nov 18, 2016

Published [email protected]

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

Successfully merging a pull request may close this issue.

3 participants