-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(utils): add to JSONSchema4Type missing Array and Object #7406
fix(utils): add to JSONSchema4Type missing Array and Object #7406
Conversation
Thanks for the PR, @wespickett! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
1ae82b7
to
5b9890e
Compare
5b9890e
to
d2bd0e1
Compare
As per our contributing guide please file an issue first so that we can discuss the change and the usecases you're intending to target with it. Closing pending discussion. |
packages/utils/src/json-schema.ts
Outdated
| JSONSchema4Array | ||
| JSONSchema4Object |
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 don't think it is the best thing for us to add this here because JSONSchema4Type
is used in the enum
property where arrays and objects are disallowed (slash non-sensical).
So I think it makes sense for us to define a new type JSONSchema4DefaultValue
which includes all of the valid default values.
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.
@bradzacher Updated with a separate type named JSONSchema4TypeExtended
, which I thought could be a better name, but can change it to JSONSchema4DefaultValue
if needed
@@ -460,7 +476,7 @@ export interface JSONSchema4NumberSchema extends JSONSchema4Base { | |||
/** | |||
* @see https://json-schema.org/understanding-json-schema/reference/boolean.html | |||
*/ | |||
export interface JSONSchema4BoleanSchema extends JSONSchema4Base { | |||
export interface JSONSchema4BooleanSchema extends JSONSchema4Base { |
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.
I'll leave this as a future exercise - but we can go one step further with this and narrow the default prop for each type. Eg a boolean type schema can only have a boolean default, and an array type schema can only have an array.
Thanks for fixing this!
Updates type to add missing Array and Object
From https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/json-schema/index.d.ts#L30
PR Checklist
Overview