Skip to content
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

Account for properties and additionalProperties in allOf #207

Conversation

msutkowski
Copy link

@msutkowski msutkowski commented Feb 5, 2022

Hi there! We've had a handful of issues reporting inaccurate type generation for users of RTK Query Code Generator which leverages oazapfts.

I made a quick reproduction comparing oazapfts output to a user-provided schema to various other generators here: https://github.com/msutkowski/oazapfts-typesecript-codegen-repro. You can see that other generators will produce a type like:

type Dog = Animal & { uniqueDogProp?: string | null }.

With the current version of oazapfts, we were generating

type Dog = Animal

This PR addresses that by accounting for properties and additionalProperties when handling the schema.allOf case. The diff in the output looks like this against the repro above:

image

Referenced issues with reports:

@msutkowski
Copy link
Author

Ah, should've checked before I opened this - but it's a duplicate of #120

@msutkowski
Copy link
Author

Just noting for anyone else that might be following this on RTKQ's behalf - we're going to start maintaining a patched version of this for issues that impact our users directly.

Copy link
Collaborator

@jonkoops jonkoops left a comment

Choose a reason for hiding this comment

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

Other changes look good. If we can get rid of the Prettier config I think we can merge this.

.prettierrc Outdated
@@ -0,0 +1,4 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove the Prettier config from this PR? Seems out of scope for the changes here.

Copy link
Author

Choose a reason for hiding this comment

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

removed.

@Xiphe Xiphe self-assigned this Nov 8, 2022
@msutkowski msutkowski force-pushed the handle-additional-properties-in-intersection branch from b3ef644 to e4ff774 Compare November 8, 2022 16:24
@msutkowski msutkowski closed this Nov 8, 2022
@msutkowski msutkowski force-pushed the handle-additional-properties-in-intersection branch from e4ff774 to 765112a Compare November 8, 2022 16:25
@Xiphe
Copy link
Collaborator

Xiphe commented Nov 8, 2022

oh, were the changes here also part of #234 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants