-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
modernize tooling: 2024 edition #576
Conversation
in order to allow for more efficient package handling - remove husky and lint-staged in favor of linting in ci in order to reduce tooling packages required - switch package to type=module - switch to vite in order to provide cjs and mjs entries - switch to vitest less config, less packages, faster - introduce packages/ folder #monorepostyle BREAKING CHANGE: package has been split and imports have been cleaned up old | new ---------------------------|-------------------------- oazapfts/runtime | @oazapfts/runtime oazapfts/lib/runtime | @oazapfts/runtime oazapfts/runtime/query | @oazapfts/runtime/query oazapfts/lib/runtime/query | @oazapfts/runtime/query - new - | @oazapfts/runtime/headers - new - | @oazapfts/runtime/util oazapfts/codegen | oazapfts oazapfts/lib/codegen | oazapfts non-generated code using oazapfts types and imports will most likely need to be migrated generated clients should now have @oazapfts/runtime as dependency instead of oazapfts `handle`, `ok`, `okify` and `optimistic` runtime helpers have been moved to `@oazapfts/runtime` along with other exports that mainly have been used internally. When you miss something from `oazapfts` package, try importing it from `@oazapfts/runtime` ref #568 fix #401
Pre-released as |
Would appreciate if someone could give this a review. As this is quite a pack I'd also be available to pair it. 🤙 |
Was able to update one of my clients that I'm maintaining pretty seamlessly 🤙 |
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 noticed a bug in the cjs build that i've pointed out here, but otherwise it seems good?
I have an experimental branch for our work repo where i've updated to the beta version, and everythng runs well.
.github/workflows/ci.yml
Outdated
@@ -14,8 +14,24 @@ jobs: | |||
node-version: 20 | |||
check-latest: true | |||
- run: npm ci | |||
- run: npx prettier -c . |
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.
would it be better to make an "npm run lint" command so people can run this locally as well?
import { OpenAPIV3, OpenAPIV3_1 } from "openapi-types"; | ||
import * as cg from "./tscodegen"; | ||
import generateServers, { defaultBaseUrl } from "./generateServers"; | ||
import { Opts } from "."; | ||
|
||
const factory = ts.factory; |
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.
This line appears to be breaking the cjs build but not the esm build if you are attempting to run the code generator programatically.
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.
oh, good catch! I'm gonna have a look 👍
Thanks for reviewing! Highly appreciated 🫶
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.
beta.2 fixes this.
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.
Awesome!
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.
lgtm! thanks again for all your work on this library
this caused typescript imports to fail in cjs bundle ref #576
and emoji to ci steps 😍 ref #576
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Awesome! Thank you both for the feedback and suggestions! |
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.
🎉 This PR is included in version @oazapfts/runtime-v1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎉 This PR is included in version oazapfts-v6.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Main motivation behind this was to split
@oazapfts/runtime
into a standalone package in order to allow for more efficient package handling.While at it I moved to
"type": "module"
and made it so we provideesm
andcjs
builds.BREAKING CHANGE:
package has been split and imports have been cleaned up
non-generated code using oazapfts types and imports will most likely need to be migrated
generated clients should now have @oazapfts/runtime as dependency instead of oazapfts
handle
,ok
,okify
andoptimistic
runtime helpers have been moved to@oazapfts/runtime
along with other exports that mainly have been used internally. When you miss something fromoazapfts
package, try importing it from@oazapfts/runtime
ref #568
fix #401