-
-
Notifications
You must be signed in to change notification settings - Fork 528
Improve composition #1383
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
Improve composition #1383
Conversation
|
ddef580
to
86d5889
Compare
Deploying with
|
Latest commit: |
3df822b
|
Status: | ✅ Deploy successful! |
Preview URL: | https://7e0756e4.openapi-ts.pages.dev |
Branch Preview URL: | https://improve-composition.openapi-ts.pages.dev |
5cb1664
to
95a3bcc
Compare
95a3bcc
to
3df822b
Compare
"required" in schemaObject && | ||
Array.isArray(schemaObject.required) | ||
) { | ||
allOf = tsWithRequired( |
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.
Change: don’t indiscriminately add WithRequired<…>
; instead, only add it when it’s actually needed, and with the proper keys
) { | ||
// don’t try and make keys required if the $ref doesn’t have them | ||
const validRequired = (required ?? []).filter( | ||
(key) => !!resolved.properties![key], |
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 is probably brittle across multiple nested compositions, but that’s OK for now. I don’t think that’s a common scenario to need to specify required
keys on a different schema object from a deeply-nested allOf
chain
// options: DEFAULT_OPTIONS, | ||
}, | ||
], | ||
[ | ||
"allOf > core properties", |
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.
New test
Changes
Fixes
allOf
bug raised in #1368, and adds tests for this improvement in behavior. No changeset because this is likely a regression caused from the rewrite.How to Review
Tests should pass
Checklist
docs/
updated (if necessary)pnpm run update:examples
run (only applicable for openapi-typescript)