tarn.js icon indicating copy to clipboard operation
tarn.js copied to clipboard

Wait for pending validations instead of checking on interval

Open jgautier opened this issue 5 years ago • 5 comments

After looking through the code I thought it might be better to wait for the promises to resolve instead of checking the count on a timer. I also added a regression which I tested by checking out the 2.0.0 version of tarn.js and writing this test that passes, then I checked out the version before my other fix and see that it failed, and now it is passing on this commit and the previous fix.

jgautier avatar Jun 04 '20 16:06 jgautier

There might have been a reason why I didn't write the code like that originally... but looks like all the tests are passing so this should be good. Anyways I need to look to this more carefully, before merging.

elhigu avatar Jun 04 '20 18:06 elhigu

I might have been worried about race conditions, where some of the validations are added to the array later on because of some delay somewhere or something like that. So at least code creating those validation promises should be looked through and checked that it is not possible that more validations are added to that array if .destroy() has been called.

elhigu avatar Jun 04 '20 18:06 elhigu

@elhigu Should we rebase and merge this?

kibertoad avatar Jan 16 '21 22:01 kibertoad

validations are added to the array later on because of some delay somewhere or something like that

Probably the part of code that worried me is going to change to be more robust. Better not to merge this yet.

elhigu avatar Jan 17 '21 10:01 elhigu

@elhigu Where are we standing with this?

kibertoad avatar Jul 11 '22 05:07 kibertoad