-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Update TypeBox to 0.26.4 #64
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
"fastify": "^4.0.0" | ||
}, | ||
"dependencies": { | ||
"@sinclair/typebox": "^0.26.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this dependency is moved from peerDependencies
to dependencies
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i see the issue discussion, personally disagree with this approach.
a dependency should never exposed from a wrapper library.
@sinclair/typebox
is "unstable" since they don't tag a stable version (>=1.0.0) yet so they minor can mutate public interface, but hotfix must don't change them. so the solution is to allow only versions that we know are compatible with this package.
using Tilde
instead of Caret Ranges
https://github.com/npm/node-semver#tilde-ranges-123-12-1 to indicate what version of @sinclair/typebox
this library support.
in this way if @sinclair/typebox
change public interfaces during upgrade from 0.25.*
to 0.26.*
this library trigger an alert when final user install an incompatible version and the library have more control about the supported version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sinclairzx81 seems like |
@benevbright Hi. Yeah, value conversion should be supported for the |
@sinclairzx81 one thing is that
|
@benevbright Hi, The update only implemented value coercion (so If you need |
yeap. sorry. I meant it works when using |
This PR updates TypeBox to
0.26.4
and re-exports theType.*
builder on the provider. This PR should resolve the following two issues.Updates:
peerDependencies
todependencies
Type
module via barrel export on ProviderType
import on Provider.TypeBoxValidatorCompiler
with new automatic value coercion for headers, querystring and params only (this is new functionality available in 0.26.0)TypeBoxValidatorCompiler
value coercion.Notes:
@mcollina @RafaelGSS Currently all unit tests are passing, however the code coverage checks are reporting below threshold when re-exporting TypeBox on the Provider. I'm not sure the best way to configure the threshold, or have the coverage checks ignore the TypeBox re-export. Will mark this PR as editable for other contributors to take a look at.
Submitting for review