-
Notifications
You must be signed in to change notification settings - Fork 78
feat(api-gen): enhance schema merging #2126
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CodSpeed Performance ReportMerging #2126 will create unknown performance changesComparing Summary
Footnotes
|
patzick
left a comment
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.
Nice addition, currently during override you should specify _DELETE_ value in $ref for the same effect. What if it's the other way around and you want to have $ref instead of composition key?
One thing is that it's missing test scenarios for it so than it would make it also properly documented and easier to understand
| // biome-ignore lint/performance/noDelete: delete $ref | ||
| delete obj.$ref; |
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.
probably that would not trigger linter error
| // biome-ignore lint/performance/noDelete: delete $ref | |
| delete obj.$ref; | |
| delete obj["$ref"]; |
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 will trigger lint/complexity/useLiteralKeys
| } | ||
|
|
||
| // remove $ref from the target object | ||
| if ( |
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's the difference here? from previous condition? I'd love to see test scenarios for it
| const compositionKeywords = ["oneOf", "anyOf", "allOf", "not"]; | ||
| if ( | ||
| typeof key === "string" && | ||
| compositionKeywords.includes(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.
the condition is really hard to get through, please maybe move some parts to the helper funcion to be used later on, maybe event the whole condition - then we can write an additional unit test 😍 for that?
const compositionKeywords = ["oneOf", "anyOf", "allOf", "not"];
const hasCompositionKeyword = (val: unknown): boolean => {
if (typeof val !== "object" || val === null || Array.isArray(val)) {
return false;
}
return compositionKeywords.some((keyword) => keyword in val);
};| if ( | ||
| typeof key === "string" && | ||
| compositionKeywords.includes(key) && | ||
| typeof obj === "object" && | ||
| obj !== null && | ||
| "$ref" in obj | ||
| ) { |
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.
then:
| if ( | |
| typeof key === "string" && | |
| compositionKeywords.includes(key) && | |
| typeof obj === "object" && | |
| obj !== null && | |
| "$ref" in obj | |
| ) { | |
| if ( | |
| typeof obj === "object" && | |
| obj !== null && | |
| "$ref" in obj && | |
| hasCompositionKeyword(value) | |
| ) { |
Prevents conflicts when merging overrides that use composition keywords. In OpenAPI, an object cannot have both $ref and composition keywords, so this removes $ref when composition is present.
closes: #2125
Schema patching improvements
patchJsonSchema.tsto remove$reffrom schema objects when an override uses composition keywords (oneOf,anyOf,allOf,not), preventing conflicts and ensuring correct schema merging. [1] [2]Code consistency and formatting
patchJsonSchema.tsfor consistency, such as removing trailing commas and ensuring uniform argument formatting. [1] [2] [3] [4] [5] [6] [7]