Skip to content

fix: openapi3spec update get schema objects #248

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: openapi3spec update get schema objects #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66da3b8
updated to include dependencies for upstream use
tom-arnold Sep 29, 2021
962e80a
changed type for getSchemaObjects to ref v3
tom-arnold Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/openapi-validator/lib/classes/OpenApi3Spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export default class OpenApi3Spec extends AbstractOpenApiSpec {
return { components: this.getComponentDefinitions() };
}

getSchemaObjects(): OpenAPIV3.ComponentsObject['schemas'] {
getSchemaObjects(): Pick<OpenAPIV3.ComponentsObject, 'schemas'> {
Copy link
Collaborator

@rwalle61 rwalle61 Oct 2, 2021 •
edited
Loading

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.

Copy link
Collaborator

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) or toSatisfySchemaInApiSpec(foo)?
  • What exactly triggers this error?
  • What's your full stack trace?
  • What's your full tsconfig?
  • What OpenAPI doc version are you using?

return this.getComponentDefinitions().schemas;
}
}
4 changes: 3 additions & 1 deletion packages/openapi-validator/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"dist"
],
"dependencies": {
"@types/axios": "^0.14.0",
"@types/request": "^2.48.7",
"axios": "^0.21.4",
Comment on lines +40 to +42
Copy link
Collaborator

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.

Copy link
Author

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.

Copy link
Collaborator

@rwalle61 rwalle61 Oct 11, 2021 •
edited
Loading

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))

"combos": "^0.2.0",
"fs-extra": "^9.0.0",
"js-yaml": "^4.0.0",
Expand All @@ -48,7 +51,6 @@
"devDependencies": {
"@types/fs-extra": "^9.0.12",
"@types/js-yaml": "^4.0.3",
"@types/request": "^2.48.7",
"@types/superagent": "^4.1.12",
"@types/typeof": "^1.0.0",
"openapi-types": "^9.2.0"
Expand Down