Skip to content

Conversation

@davidnormo
Copy link

@davidnormo davidnormo commented Mar 21, 2018

Following on from #45

All credit for the async quick-check implementation goes to promesa-check ⭐️

This PR adds this behind a new function: checkAsync. This acts exactly the same as check except that the property function needs to return a promise.

Ideally I'd prefer check to be able to accept both sync and async properties, which I may work on, but thought I'd share this for some early feedback

@leebyron
Copy link
Owner

Thanks for doing this! Very cool to see the async check implementation out there, that definitely simplifies some things.

Some next steps I can see:

  • Looks like check and checkAsync are largely the same - perhaps there is some code reuse to be had?
  • Tests are pretty sparse - I'd love to see tests for returning false, throwing an error, and rejected promises.
  • It would be really cool if check operated on sync or async properties, returning a Promise in the case of async properties, though perhaps that could be saved for a future PR.
  • Test runner integrations here would be nice as well - let's make sure that async or (done) tests in appropriate runners do the right thing.
  • We'll want to make sure the flow and typescript type definitions (found in /type-definitions) are up to date with this change

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 this pull request may close these issues.

2 participants