-
Notifications
You must be signed in to change notification settings - Fork 39
feat(loader): support validate with standard schema
#237
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: ysknsid25 <[email protected]>
validate with standard schema
Signed-off-by: ysknsid25 <[email protected]>
|
@ysknsid25 i will put more time on this PR before releasing in next minor releases. It is a big feature! |
| await expect( | ||
| loadConfig({ | ||
| configFile: "CUSTOM", | ||
| it("load fixture config with validate for zod - toThrow", async () => { |
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.
We have many new tests. As this feature is based on standard schema, validating against one of them is enough to make maintenance easier
package.json
Outdated
| "test:types": "tsc --noEmit" | ||
| }, | ||
| "dependencies": { | ||
| "@standard-schema/spec": "^1.0.0", |
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.
We can copy-paste types (as they suggest) to avoid extra dependency
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 fixed it 👍
src/types.ts
Outdated
| configFileRequired?: boolean; | ||
|
|
||
| schema?: S; | ||
| validate?: (schema: S, input: ResolvedConfig<T, MT>) => void; |
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.
Thinking how we could make it simpler. Like setting a schema could be enough (to also cover validate + schema transform)
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.
StandardSchema already has vendor and validate, so it looks like we can look at vendor and isolate the call to validate 👍
Give me more time 🙏
Signed-off-by: ysknsid25 <[email protected]>
define StandardSchema Signed-off-by: ysknsid25 <[email protected]>
define StandardSchema Signed-off-by: ysknsid25 <[email protected]>
define StandardSchema Signed-off-by: ysknsid25 <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
==========================================
+ Coverage 76.92% 77.74% +0.82%
==========================================
Files 7 5 -2
Lines 806 346 -460
Branches 80 147 +67
==========================================
- Hits 620 269 -351
+ Misses 184 64 -120
- Partials 2 13 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9b10eda to
cfdf5fa
Compare
implement validate and transform Signed-off-by: ysknsid25 <[email protected]>
cfdf5fa to
f1758fa
Compare
|
Hello @pi0 . I have a consultation 🙇 Can you make feature branch for this issue? In this branch, write test for Zod and valibot and marge to new feature branch. remaning test code will be written another each branch. How about? 👀 |
attempts to resolve #228
Test Targets
This is because they are clearly stated to conform to Standard Schema.