Fix: clear pending scrollTracker's timeout when the instance is destroyed #339
Conversation
|
@sqal thanks for this. I can't imagine you saw it, but I mentioned in a recent PR that I was thinking of dropping this async behaviour completely: #330 (comment). This would simplify the code and flow of execution, and also avoid the kind of bug you've spotted here. So with that in mind, I wonder whether it's worth just dropping setTimeout rather than fixing it up as you have done here. @hacknug did you start on this by any chance? |
|
@WickyNilliams not yet, got caught up with client work 🥴 |
|
No worries! Just wanted to be sure there wasn't any duplication of effort. I think I'll do this today, unless you're particularly keen on it? I don't mind waiting if so. Edit: I just went ahead and did it. @hacknug, do i remember rightly, you have access to an iOS device? If so, it'd be great if you could look at the various iOS bugs |
|
Go ahead. I'm in between projects and I'm not sure how much time I'll have to put in oss this week. Sorry! |
|
Not a problem. As I'm sure you're aware, my participation in OSS is inconsistent and infrequent (to say the least haha), there's no expectation of any more than that :) |
|
@WickyNilliams No, I hadn't seen your comment in the other PR before I opened mine. I guess we can close this PR now that you've done refactoring and this bug should be fixed? :) |
|
@sqal yes, this can be closed now as i've just merged the PR removing the async behaviour. Thanks for highlighting this issue anyway, I really appreciate you taking the time to contribute :) |
Fixes #338