-
Notifications
You must be signed in to change notification settings - Fork 5.1k
6156 find replacement for is my json valid #6264
Conversation
Bundle StatsHey there, this message comes from a github action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Deploying with
|
Latest commit: |
7642563
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2d4abcf8.web3-js-docs.pages.dev |
Branch Preview URL: | https://ok-6156-find-replacement-for.web3-js-docs.pages.dev |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## 4.x #6264 +/- ##
=======================================
Coverage 88.71% 88.72%
=======================================
Files 198 198
Lines 7638 7608 -30
Branches 2103 2094 -9
=======================================
- Hits 6776 6750 -26
+ Misses 862 858 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
packages/web3-validator/src/types.ts
Outdated
@@ -143,10 +143,7 @@ export type Schema = { | |||
}; | |||
export interface Validate { | |||
(value: Json): boolean; | |||
errors?: ValidationError[]; | |||
errors?: ZodIssueBase[]; |
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.
This will be breaking change, as Validation interface is being exported and available to users. For this during release we will need to bump major version of validator package. so:
- mention this in changelog
- I think create some validation error wrapper around
ZodIssueBase
, so we are not tied to specific lib (like we were is-my-json-valid.ValidationError)
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.
ValidationError wrapper was added
LGTM, just need some minor changes. Plus with zod lib using in this PR, could you manually test some recent issues reported for browser support due to is my json valid lib. |
|
||
return false; | ||
// type === number | ||
return value === 1 || value === 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.
here can be only number type. we don't need extra conditions.
* change internal validation to jsonschema lib * implement validation with zod * load test * add load test * add changelog * increase test coverage * fix test timing * fix test timing * fix timing * re-run tests * add ValidationError wrapper * add json-schema * add change log * fix change log * revert type changes * increase coverage * changelog sync
Description
Fixes #6156 and #6168
Please include a summary of the changes and be sure to follow our Contribution Guidelines.
Type of change
Checklist:
npm run lint
with success and extended the tests and types if necessary.npm run test:unit
with success.npm run test:coverage
and my test cases cover all the lines and branches of the added code.npm run build
and testeddist/web3.min.js
in a browser.CHANGELOG.md
file in the root folder.