-
Notifications
You must be signed in to change notification settings - Fork 64
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
feat: yaml options #144
feat: yaml options #144
Conversation
src/dsl/openapi-builder30.ts
Outdated
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
replacer?: any[] | ((key: any, value: any) => unknown) | null, | ||
options?: | ||
| string | ||
| number | ||
| (yaml.DocumentOptions & | ||
yaml.SchemaOptions & | ||
yaml.ParseOptions & | ||
yaml.CreateNodeOptions & | ||
yaml.ToStringOptions) |
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'd suggest to make a reference to the types of the yaml.stringify()
parameters instead
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.
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |
replacer?: any[] | ((key: any, value: any) => unknown) | null, | |
options?: | |
| string | |
| number | |
| (yaml.DocumentOptions & | |
yaml.SchemaOptions & | |
yaml.ParseOptions & | |
yaml.CreateNodeOptions & | |
yaml.ToStringOptions) | |
replacer?: Parameters<typeof yaml.stringify>[1], | |
options?: Parameters<typeof yaml.stringify>[2] |
then you'd not need:
- to reproduce internal typing of another module
- to mute ESLint error
That will also make it future resistant (maintainability) if yaml
dependency upgrades with changed types, @urugator
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 wanted to do that, but it's is not doable for overloaded functions, because TS doesn't know which overload to pick.
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.
pick the one having replacer
, consider Extract
for that, @urugator
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.
Dunno what you mean with the Extract
, but turns out Parameters
are parameters of the last signature, which is the one we need. So unless they swap or introduce new signatures it should be fine I guess.
src/dsl/openapi-builder31.ts
Outdated
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
replacer?: any[] | ((key: any, value: any) => unknown) | null, | ||
options?: | ||
| string | ||
| number | ||
| (yaml.DocumentOptions & | ||
yaml.SchemaOptions & | ||
yaml.ParseOptions & | ||
yaml.CreateNodeOptions & | ||
yaml.ToStringOptions) |
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.
Changelog.md
Outdated
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'd say it's too much changes to this file that are unrelated to the proposed feature.
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.
It's done by prettier, should I add Changelog.md to prettier ignore?
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.
suggesting to use Parameters<typeof yaml.stringify>
https://www.typescriptlang.org/docs/handbook/utility-types.html#parameterstype
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.
nice
Types are copied from
yaml
repo:To keep things simple and in line with
getSpecAsJson
the signature with options only (without replacer) is not supported.Fixes: #143