-
Notifications
You must be signed in to change notification settings - Fork 36
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: openapi3spec update get schema objects #248
base: master
Are you sure you want to change the base?
fix: openapi3spec update get schema objects #248
Conversation
Codecov Report
@@ Coverage Diff @@
## master #248 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 421 421
Branches 58 60 +2
=========================================
Hits 421 421
Continue to review full report at Codecov.
|
@@ -88,7 +88,7 @@ export default class OpenApi3Spec extends AbstractOpenApiSpec { | |||
return { components: this.getComponentDefinitions() }; | |||
} | |||
|
|||
getSchemaObjects(): OpenAPIV3.ComponentsObject['schemas'] { | |||
getSchemaObjects(): Pick<OpenAPIV3.ComponentsObject, 'schemas'> { |
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.
Thanks @tom-arnold for raising this and submitting a fix! 🙂
I can't reproduce the error. Does this error come from jestOpenAPI(spec)
or toSatisfySchemaInApiSpec(foo)
? Would you mind showing how you're triggering this error and your full stack trace? Also your tsconfig and OpenAPI doc version please.
For fastest resolution, please recreate this in our recreation template.
I ask because I'm not sure how your change fixes the error. I assume you want '{ [key: string]: ReferenceObject | SchemaObject; } | undefined'
to change to '{ [key: string]: ReferenceObject | SchemaObject; }'
.
But given OpenAPIV3.ComponentsObject
has type:
interface ComponentsObject {
schemas?: {
[key: string]: ReferenceObject | SchemaObject;
};
OpenAPIV3.ComponentsObject['schemas']
is { [key: string]: ReferenceObject | SchemaObject; } | undefined
,
whereas Pick<OpenAPIV3.ComponentsObject, 'schemas'>
is
{
schemas?: {
[key: string]: ReferenceObject | SchemaObject;
};
}
which does not match the actual value this.getComponentDefinitions().schemas
. This seems wrong to me, so I wonder if it fixes your error by breaking something, so TypeScript accepts any
.
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.
@tom-arnold I saw this error when I enabled TypeScript's strict mode, so I wonder if it will be fixed by https://github.com/openapi-library/OpenAPIValidators/pull/254/files#diff-f24b64aecef0d95e0a629f3f7ba7ea12fd4f2ef959ec61b82448a54fe309bbe2R15.
It'd be great to know exactly what error you're seeing:
- does this error come from
jestOpenAPI(spec)
ortoSatisfySchemaInApiSpec(foo)
? - What exactly triggers this error?
- What's your full stack trace?
- What's your full tsconfig?
- What OpenAPI doc version are you using?
"@types/axios": "^0.14.0", | ||
"@types/request": "^2.48.7", | ||
"axios": "^0.21.4", |
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.
Thanks, I accidentally didn't include axios
in openapi-validator
's package.json 🙂 I guess it doesn't break anything because we use it only for types.
Since we only use it for types, it should be in devDependencies
. Same for @types/request
.
Also axios
already has the types, so @types/axios
is deprecated.
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.
Good point, @types/axios
is not necessary as it is depreciated. However, it does start to break things upstream when trying to convert typescript to javascript using tsc
. The error message I get without axios
is the following:
node_modules/openapi-validator/dist/classes/AxiosResponse.d.ts:1:57 - error TS2307: Cannot find module 'axios' or its corresponding type declarations.
1 import type { AxiosResponse as AxiosResponseType } from 'axios';
As for @types/request
, I will have to look into that.
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.
Yep we'll get that error if we don't have axios
, because the types are from axios
. We shouldn't need @types/axios
though.
I'm quite curious as to why this error triggers when you run tsc
, so it'd be great to know exactly how the error is triggered (#248 (comment))
When using the jest-openapi package in on of our projects, our build is failing with the following message when running tsc:
After reviewing the current code, it looks like the issue lies in the OpenApi3Spec.d.ts file, specifically that the current implementation of
getSchemaObjects()
is using the wrong type. By making this change, I was able to successfully complete our build.System: Kali-linux WSL2
typescript: v 4.4.3
node: v 14.17.2