Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jest-validate does not work well with async functions #7894

Closed
ehmicky opened this issue Feb 14, 2019 · 5 comments · Fixed by #7896
Closed

jest-validate does not work well with async functions #7894

ehmicky opened this issue Feb 14, 2019 · 5 comments · Fixed by #7896

Comments

@ehmicky
Copy link
Contributor

ehmicky commented Feb 14, 2019

🐛 Bug Report

When validating input using jest-validate's opts.exampleConfig, the validation distinguishes between sync and async functions which creates three issues.

First:

  • if opts.exampleConfig uses an async function, functions returning a promise will make the validation fail, but async functions will pass. However one might expect that those two are functionally equivalent.
  • if opts.exampleConfig uses a non-async function, functions returning a promise will make the validation pass, but async functions will fail.
  • multipleValidOptions() can be used to work around this problem, but I expect very few users will do so.

Second: when using Babel async function are transpiled to non-async function, changing the behavior of jest-validate. Also this means jest-validate always validate against async functions when the code is using Babel.

Third: the error message is not descriptive enough as it says function for both sync and async functions.

Note that the current type checking is using Object.prototype.toString, which returns either '[object Function]' or '[object AsyncFunction]'.

To Reproduce

const { validate } = require('jest-validate')
validate({ name: async () => {} }, { exampleConfig: { name: () => {} } })
/*
  ● Validation Error:

  Option "name" must be of type:
    function
  but instead received:
    function

  Example:
  {
    "name": () => {}
  }
*/

Expected behavior

If a sync function is passed to exampleConfig, async functions should pass.

If an async function is passed to exampleConfig, sync functions should pass.

Link to repl or repo (highly encouraged)

https://repl.it/repls/WelcomeFrizzyService

Run npx envinfo --preset jest

  System:
    OS: Linux 4.18 Ubuntu 18.10 (Cosmic Cuttlefish)
    CPU: (12) x64 Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz
  Binaries:
    Node: 11.9.0 - ~/.nvm/versions/node/v11.9.0/bin/node
    Yarn: 1.12.3 - ~/.nvm/versions/node/v11.9.0/bin/yarn
    npm: 6.8.0 - ~/.nvm/versions/node/v11.9.0/bin/npm
@thymikee
Copy link
Collaborator

You're right, we don't have a separation of sync/async functions in jest-validate. I'm happy to review a PR fixing current behavior :)

@ehmicky
Copy link
Contributor Author

ehmicky commented Feb 14, 2019

@thymikee this is the opposite: there is a separation of sync/async functions in jest-validate, where there probably should not.

As described above this creates issues with Babel transpiling, or when converting from/to an async function from/to a function returning a promise (which should be equivalent).

If you're happy with the following solution, I can submit a PR: if typeof value === 'function' for both values (input value and exampleConfig value), do not make validation condition fail.

See current code.

@thymikee
Copy link
Collaborator

I'm cool with that. Just add a test :D

@ehmicky
Copy link
Contributor Author

ehmicky commented Feb 14, 2019

Done at #7896

thymikee pushed a commit that referenced this issue Feb 14, 2019
## Summary

`jest-validate` distinguishes between sync and async functions, but it should not. Fixes #7894.

## Test plan

```js
const { validate } = require('jest-validate')
assert(validate({ name: async () => {} }, { exampleConfig: { name: () => {} } }).isValid)
assert(validate({ name: () => {} }, { exampleConfig: { name: async () => {} } }).isValid)
assert(validate({ name: async () => {} }, { exampleConfig: { name: async () => {} } }).isValid)
assert(validate({ name: () => {} }, { exampleConfig: { name: () => {} } }).isValid)
```
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants