Fix Collection.closest

[juliankrispel]

At the moment Collection.closest doesn't actually work with filters. If type.check(parent.value) in the while loop on line 80 returns true it will stop the while loop, skipping the last expression, which tests if the filter condition applies.

This pr fixes this and refactors the while loop into a recursive function which is imo more readable.

I also added some specs to prove that closest is working correctly.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[fkling]

Thanks for noticing this! However, I'm not in favor of a recursive solution. It's unlikely that someone will run in to call stack issues, but still. I'm more comfortable doing this when we have TCO :)

It seems this could be easily fixed by changing the while condition to

while (
  parent &&
  !(
    type.check(parent.value) &&
    (!filter || matchNode(parent.value, filter))
  )
) {

(or something equivalent)

[juliankrispel]

@fkling my pleasure, thanks for this awesome project! I'll change it to your liking, it's your project after all, all I want to do is contribute :)

Just as a side note: I'm not strongly opinionated about this, but it certainly helped to debug the thing, also three if conditions although more verbose seemed easier to reason about than one big condition, hence I thought it was reasonable.

[juliankrispel]

@fkling changed it back to a while loop, the added tests cover the issue I noticed so it should be good to go :)

[fkling]

I squashed all the commits. This is now merged with faba497 and release as v0.3.12. Thank you again! :)

[juliankrispel]

@fkling thanks :), here's a kitten