@hapi/validate schemas in the context of hapi-swagger use

Marsup commented 3 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: 14.16.0
module version: 20.1.0
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): hapi-swagger@14.1.0
any other relevant information:

How can we help?

I'm starting an issue here, but I don't really know if it's up to hapi or hapi-swagger to fix the issue, I'd say hapi, but you may disagree. There is currently an error thrown from hapi-swagger that looks like this:

Error: Cannot mix different versions of joi schemas
    at new module.exports (./node_modules/@hapi/hoek/lib/error.js:23:19)
    at module.exports (./node_modules/@hapi/hoek/lib/assert.js:20:11)
    at Object.exports.isSchema (./node_modules/joi/lib/common.js:132:5)
    at Object.utilities.isJoi (./node_modules/hapi-swagger/lib/utilities.js:236:24)
    at Object.utilities.hasJoiChildren (./node_modules/hapi-swagger/lib/utilities.js:246:19)
    at ./node_modules/hapi-swagger/lib/paths.js:291:21
[...]

The root cause is coming from this line. When hapi-swagger is checking that all the schemas are joi objects, this one is not, so the throw is totally expected.

Now I see 2 solutions to that :

use the validator to create that schema so that it uses the correct joi version, but it doesn't seem easy to add the allow(null) part if we're not sure that we're getting a joi object back from the validator
keep the false as it is and do some special treatment during the validation, but it has imho one major drawback, it's going to be hard to throw joi-compatible errors, which could cause some problems for people catching errors in failAction or any other hook

What do you think?

kanongil commented 3 years ago

Hmm… yeah, this does seem strange for hapi to expose Validate for run-time validations. Eg. there is no way to localize the generated error.

Ideally, hapi would use the passed validator. Maybe the correct solution is to allow validators to provide a "nothing-allowed" schema method? Maybe registered something like this?

server.validator(Joi, {
    empty: Joi({}).allow(null)
});

devinivy commented 3 years ago

Eg. there is no way to localize the generated error.

keep the false as it is and do some special treatment during the validation, but it has imho one major drawback, it's going to be hard to throw joi-compatible errors

I wonder if this drawback can be addressed by users using their validator rather than the convenient false shorthand, if it's important to them. If that is the case, then I think the second option ("keep the false as it is and do some special treatment") would be a nice way to handle it. We may be able to go a step further and give some basic guarantees about the error format for this case.

hapijs / hapi