preact
preact copied to clipboard
Event bubbling can break in test conditions
- [x] Check if updating to the latest Preact version resolves the issue
Describe the bug In test scenarios, on a fast machine, it's possible for preact rendering and a subsequent programmatic call to occur within the span of a single millisecond (yay!). However, that can trip up the logic in https://github.com/preactjs/preact/pull/4126 because the _dispatched and _attached flags in the dispatched events and associated event listener props all end up getting the same value from Date.now(), resulting in any bubbling listeners getting short-circuited.
To Reproduce
Steps to reproduce the behavior:
- Go to https://codesandbox.io/s/infallible-https-2z9kls?file=/src/index.js and view the console.log
The test case loops 50 times, rendering a div containing an input, both with a 'focusin' listener, and immediately calling focus on the input. The listener on the input element is always called (since the new logic always invokes on the first listener), but on my machine, the listener on the div is usually not called.
Expected behavior If you flip the preact version back to 10.17.1, you'll see 50 perfectly interleaved calls to the input + div.
Not sure if the spirit of the original fix could be maintained by just using an incrementing counter rather than Date.now() which I think would solve this case as well?
it might be worse - afaik the resolution of Date.now() is more granular depending on an number of factors. in firefox it can e.g. be up to 100ms (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/now) and it might depend on the underlying operating system, too.
i used a tuple of a Date millisecond and an auto-incrementing 32 bit int (number|0) in a similar case. of course this is not a "real" solution because as soon as someone manages to do 2^32 operations within the same date granule, the problem will come back. it's much more unlikely, though.
p.s. sorry i didn't raise this when i first saw the PR being merged - i thought it might happen but didn't want to be a smartass :/
Thanks for the additional context! I can confirm that running my testcase in Firefox with privacy.resistFingerPrinting enabled, the bubbling almost always fails to work (48+ of the 50 runs).
Those are all very valid points, I am going to give it some thought whether we can have a better solution to this 😅 basically something where we can indicate that a dom-node/event-handler was created later than when this event started bubbling up.
In real user scenario's this probably works out pretty well, however for this case in particular, the programmatic one it's a bit harder. In real interactions 100ms isn't even a problem imho
@JoviDeCroock Is there any chance that Date.now() can be replaced with performance.now()? This change will not resolve the issue completely but it will improve the chances of the correct behavior. I've just tried it with the above code sample. I can still see some failures to bubble up the event, but it succeeds in most of the cases.
performance.now() is significantly slower, we might need to experiment with some counter approach where the counter increments upon render flushes 😅 maybe a similar approach to this where it gets cached 😅
I haven't had much time to look into this but I acknowledge that issues in the computed scenario's are annoying. It however does solve a lot of real world scenario's from component libraries
Also feel free to look into this, the issues fixed are linked and should have adequate reproductions!