Schema assert in Joi extension args are not using updated value from validation
- Dominant language
- JavaScript
- Stars
- 21.2k
- Forks
- 1.5k
- Avg merge
- 4h 57m
- Merged PRs (30d)
- 14
Description
#### 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*: v12.18.3
* *module version with issue*: v17.2.1
* *last module version without issue*: v15
* *environment* (e.g. node, browser, native): node
* *used with* (e.g. hapi application, another framework, standalone, ...): hapi
* *any other relevant information*: The problem was noticed in confidence in the hapipal org.
#### What are you trying to achieve or the steps to reproduce?
In Confidence when using Joi@15 we used to validate extension rules arguments using the `assert` property with a Joi schema. In that schema we were using a [`.default`](https://github.com/Nargonath/confidence/blob/d83de302ee2b06dda4d4cffc68d30fe8a1c7c280/lib/schema.js#L38) and it worked. The rule argument was initialized with the default when value was not given.
We recently updated to the latest v17 and we noticed that this was no longer the case when switching to the new extension API. I believe this is a breaking change that was not documented in Joi@16 release notes but I may be wrong. At least I didn't find it there FWIW. I think the problem comes from [here](https://github.com/sideway/joi/blob/f309431e17bce06c3d4e2ace35ed35cbabd01799/lib/common.js#L200) since the value resulting from the validation is not used. Compared to [v15](https://github.com/sideway/joi/blob/b604775fccad77836b000aec2b33cdb6de26a73a/lib/index.js#L387) where it seems that the value from the arguments validation is used afterwards, i'm not entirely sure that's the right line though.
#### What was the result you got?
The `.default` was not applied hence the subsequent [conditions](https://github.com/Nargonath/confidence/blob/d83de302ee2b06dda4d4cffc68d30fe8a1c7c280/lib/schema.js#L53) in the `validate` function based on the assumption that it was applied did not work anymore.
#### What result did you expect?
I expected the `.default` to be applied.
My point is to bring awareness on this issue but I don't know if it's an intended breaking change that just got missed during the changelog write-up or whether it's an unintended breaking change. Depending on the situation we could look into either add a mention in the v16 release note issue about it or see whether that's a behavior you'd want to bring back or not. Either way I'd be happy to help.
Contributor guide
Assessment
This issue has not been assessed yet.