-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
improve types #12
improve types #12
Conversation
src/superstruct.ts
Outdated
@@ -31,7 +20,7 @@ const parseErrorSchema = ( | |||
currentPath, | |||
validateAllFieldCriteria, | |||
previous, | |||
type, | |||
type!, |
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.
@bluebill1049 type
may be undefined
.
using !
to ignore type errors, how should I fix this?
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.
maybe we should omit the type if it can be undefined
? your thoughts?
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.
@bluebill1049 If we omit type, I think you need to fix FieldError
type as follows.
export type FieldError = {
- type: string;
+ type?: string;
ref?: Ref;
types?: MultipleFieldErrors;
message?: Message;
isManual?: boolean;
}
I think it's better to use an empty string (''
) to avoid this fix.
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.
oh that's right, let's leave as ''
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.
I will fix this!
src/superstruct.ts
Outdated
@@ -41,7 +30,7 @@ const parseErrorSchema = ( | |||
type, | |||
...(validateAllFieldCriteria | |||
? { | |||
types: { [type]: message || true }, | |||
types: { [type!]: message || true }, |
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.
Same as above
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.
looks good, you want to publish a new patch?
Yes, I want to release a new patch! |
make sure push the tag and leave a release note as well :) 👍 |
What about the version? |
we can go |
I will release |
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.
just release a real version
npm version patch
0.0.4 👍
@bluebill1049 OK! I will do it. |
No description provided.