-
Notifications
You must be signed in to change notification settings - Fork 21
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(openapi-generator): support types imported from node_modules
#766
Conversation
de8290e
to
87fc87e
Compare
wallet-platform-types
node_modules
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'm wondering if this should count as a breaking change. Even though I haven't ran into it in testing, a package that previously didn't expect to have node_modules parsed may suddenly break upon upgrading if there is something that now causes an error in node_modules.
@@ -34,7 +35,7 @@ | |||
"@types/resolve": "1.20.6", | |||
"c8": "9.1.0", | |||
"memfs": "4.9.2", | |||
"typescript": "4.7.4" | |||
"typescript": "^4.7.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.
What necessitated this change? I think we can support this but would need to incorporate the patch here (or a version of io-ts with that patch upstreamed to 2.1.3)
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.
Yeah I don't think we need this either, must've been something I introduced when trying to get everything working locally. We can remove this 👍🏽
@@ -0,0 +1 @@ | |||
sample-types/ |
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.
Is this redundant with this entry? https://github.com/anshchaturvedi
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.
hmm yeah I agree, will check this and remove as necessary.
I gave myself the task of running all the current |
The way to do with with semantic-release is to add a line in a commit body containing |
BREAKING CHANGE: supporting `node_modules` may cause current `-types` libraries to break which use `openapi-generator`
24a7379
to
b97074e
Compare
done! 17d919a |
🎉 This PR is included in version @api-ts/[email protected] 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version @api-ts/[email protected] 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version @api-ts/[email protected] 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version @api-ts/[email protected] 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
🎉 This PR is included in version @api-ts/[email protected] 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
Ticket: DX-389
This PR aims to introduce support to parse
types
from libraries living insidenode_modules
. Previouslyopenapi-generator
only parsedtypes
that were defined locally and this heavily interfered with creating an automated solution for generating OpenAPI specifications when API's changes are introduced.This PR also includes a more sophisticated end-to-end testing strategy so that any implementations that change the logic of how we generate our documentation will be caught by our tests.