Skip to content
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

Switch oas-validator to @stoplightio/spectral #410

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

Conversation

FallingSnow
Copy link

@FallingSnow FallingSnow commented Jul 1, 2021

@stoplightio/spectral supports openapi 3.1.0 while oas-validator does not.
One noteable drawback seems to be that @stoplight.spectral is much slower but seems to have caught thing even oas-validator didn't; such as operationRef syntax errors.

Most tests are passing but some are still failing.

Signed-off-by: Ayrton Sparling <[email protected]>
Switched to Buffer.from()

Signed-off-by: Ayrton Sparling <[email protected]>
Implement a partial getLinkLocationType:
Technically 'title' type does not exist, it should be file. operationRef follows the same guidelines as $ref. See https://swagger.io/docs/specification/using-ref/
The only types that **really** exist are local, remote, and url.

Signed-off-by: Ayrton Sparling <[email protected]>
Signed-off-by: Ayrton Sparling <[email protected]>
@FallingSnow FallingSnow marked this pull request as ready for review July 1, 2021 06:55
@FallingSnow
Copy link
Author

FallingSnow commented Jul 1, 2021

I should also point out that due to operationRef uri syntax requirements, AKA using the title of the specification in links is invalid, it had to be switched to file based references. That means that link operationRef's need to be defined by file name instead. Since no filename information is passed into openapi-to-graphql I instead added a vendor extension to the spec. You can specify the document's filename by adding a info.x-filename field to the document. This filename can be used in operationRefs. You can see this addition in the example_oas.json and example_oas3.json files.

@Alan-Cha
Copy link
Collaborator

Alan-Cha commented Jul 1, 2021

@FallingSnow Thank you for this PR! Could you explain some of the reasoning behind this change? I am not very familiar with spectral and I wonder what kind of advantages it has over oas-validator.

I should also point out that due to operationRef uri syntax requirements, AKA using the title of the specification in links is invalid, it had to be switched to file based references.

Thank you for pointing this out! The proposed change to use the file name is a welcome one. Our old method was just the best workaround we could think of at the time. I think your method is much better.

@FallingSnow
Copy link
Author

The driving change behind the switch was OAS3.1 support.

Here are some reasons I can think of:

  • Has OAS 3.1.0 support.
  • Used in/built by stoplight which gives it commercial testing and funding.
  • More active

I would also guess that spectral has friendlier error messages since it's main use case is an OpenAPI design application.

oas-kit pulse:
Screenshot from 2021-07-01 11-32-42

Spectral pulse:
Screenshot from 2021-07-01 11-32-32

@Alan-Cha
Copy link
Collaborator

Alan-Cha commented Jul 8, 2021

This seems promising! @ardatan Do you have any opinions on this?

@ardatan
Copy link
Collaborator

ardatan commented Jul 8, 2021

Personally I think OAS Validation shouldn't be part of the core library but CLI maybe? But if we want to have it, this library seems better than what we have right now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants