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

Destructuring Throws Fatal #1631

Closed
shellscape opened this issue Nov 1, 2018 · 5 comments
Closed

Destructuring Throws Fatal #1631

shellscape opened this issue Nov 1, 2018 · 5 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@shellscape
Copy link

shellscape commented Nov 1, 2018

Context

  • node version: 10.11.0
  • joi version: 14.0.3
  • environment (node, browser): Node
  • used with (hapi, standalone, ...): standalone
  • any other relevant information:

What are you trying to achieve or the steps to reproduce ?

const Joi = require('joi');
const { boolean,validate } = Joi;
const schema = boolean(); // source of error
const result = validate(true, schema);

Which result you had ?

AssertionError [ERR_ASSERTION]: Must be invoked on a Joi instance.
    at new AssertionError (internal/assert.js:268:11)
    at Object.exports.assert (/user/proj/node_modules/hoek/lib/index.js:517:11)
    at internals.callWithDefaults (/user/proj/node_modules/joi/lib/index.js:31:10)
    at root.boolean.root.bool (/user/proj/node_modules/joi/lib/index.js:73:43)

What did you expect ?

Prepending Joi onto the beginning of every assertion type is rather messy (completely subjective, personal opinion) and while I absolutely dig the expressive nature of joi, I'm looking for ways to dial down the noise in code and make the schema setup a little cleaner. I was very surprised to learn that none of the assertion methods can be safely destructured from the main instance (it's not mentioned anywhere in the docs nor issues).

It's worth noting that this fails as well:

const boolean = Joi.boolean;
const validate = Joi.validate;
const schema = boolean(); // source of error
const result = validate(true, schema);

I could get nuts and implement a Proxy to make the assignment happen, but I'd rather not slow down the validation process just for the sake of some pretty code. I'm really hoping there's a way you can cater to those who want to use the assertions and destructure the method names. I'm being pushed hard to use yup for that reason alone, but I prefer the nuances of joi.

@shellscape
Copy link
Author

And for anyone curious, here's the Proxy implementation that makes destructuring work as expected:

const Joi = require('joi');

const joi = new Proxy(Joi, {
  get(o, name) {
    return o[name].bind(o);
  }
});
const { boolean, validate } = joi;
const schema = boolean();
const result = validate(true, schema);

I haven't run benchmarks on what this will do to performance, but I do know from past experience that proxying does make a dent in speed.

@kanongil
Copy link
Contributor

kanongil commented Nov 1, 2018

The returned Joi object contains state, which is required for it to work, thus requiring a bind().

The Proxy solution you propose should work quite performant, since it is only applied during the initial destructuring.

@shellscape
Copy link
Author

Thanks for replying, I suspected as much. Pending tests to confirm performance, would there be any objection to offering that capability by default in the package?

@Marsup
Copy link
Collaborator

Marsup commented Nov 11, 2018

Yes, for people transpiling it for the browser it's going to be a problem.
I'd rather add a bind() on the root object that binds all known types than do this.

@Marsup Marsup self-assigned this Nov 25, 2018
@Marsup Marsup added this to the 14.3.0 milestone Nov 25, 2018
@Marsup Marsup closed this as completed in ae6031d Nov 25, 2018
@shellscape
Copy link
Author

thanks @Marsup

@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

4 participants