Skip to content
This repository has been archived by the owner on Oct 5, 2023. It is now read-only.

feat: promises rewrite and drop support for node 10 #131

Merged
merged 2 commits into from
Jun 3, 2021
Merged

Conversation

erunion
Copy link
Member

@erunion erunion commented Jun 2, 2021

Late 2019 in rdme we lost the ability to bundle in $ref pointers spread across multiple files, and unfortunately with the way that that library is written it's difficult to use oas-normalize because it's currently based on supplying callbacks. On top of that, the swagger-parser and swagger2openapi dependencies that oas-normalize have also been rewritten to support promises, leaving this thing in the dust.

So I've fully rewritten it to be promise-based. And added full unit test coverage in the process. And removed a handful of dependencies that it didn't really need.

🚀

🧰 What's being changed?

  • 🆕  Full rewrite of the library to be promise-based.
  • 🆕  Full test coverage.
  • 🧹  Pruning back unnecessary dependencies.
  • 💀  Drops support for Node 10.

@erunion erunion marked this pull request as ready for review June 3, 2021 15:57
@erunion erunion added the enhancement New feature or request label Jun 3, 2021
@erunion erunion changed the title feat: promises and drop support for node 10 feat: promises rewrite and drop support for node 10 Jun 3, 2021
@erunion erunion requested review from kanadgupta and rahulhegdee June 3, 2021 16:05
Copy link
Member

@kanadgupta kanadgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did a pass and changes seem sound! I don't know much about this library and if anything, I'm just weary of all the different ways an OAS file could be invalid lol. Seeing the increase in test coverage is reassuring though!

Also one general question: should all the fixtures you created here exist in @readme/oas-examples? Or are you limiting that library to only valid files?

Comment on lines -29 to +28
"swagger2openapi": "^2.11.16",
"swagger2openapi": "^7.0.6",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omfg lol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been getting pinged by Dependabot about this every month when it tries to update it. 😅

Comment on lines -27 to +26
"r2": "^2.0.1",
"@apidevtools/json-schema-ref-parser": "^9.0.7",
"node-fetch": "^2.6.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hell yeah

index.js Show resolved Hide resolved
@erunion
Copy link
Member Author

erunion commented Jun 3, 2021

Also one general question: should all the fixtures you created here exist in @readme/oas-examples? Or are you limiting that library to only valid files?

Yeah that library should only have valid examples.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants