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

rtk-query-codegen-openapi: Null values not always respected #3814

Open
mattoni opened this issue Oct 19, 2023 · 10 comments · May be fixed by #4198
Open

rtk-query-codegen-openapi: Null values not always respected #3814

mattoni opened this issue Oct 19, 2023 · 10 comments · May be fixed by #4198

Comments

@mattoni
Copy link

mattoni commented Oct 19, 2023

I've recently upgraded my OpenAPI schema to 3.1, and while in most places the

type:
    - "object"
    - "null"

style seems to generate the correct code, in other places it does not.

For example, given this:
image

The generated output is:
image

Where stack is not also nullable.

The same thing happens in various other spots, such as here in the password field of this nested object:
image

The output being:
image

It appears to generate correctly in circumstances such as this:
image

Where I use an anyOf to reference another type that can also be null in this field, the generated being:
image

It appears that the generator doesn't work correctly when you have a type with multiple values, and one of them is null. I think the codegen should support this since it is the recommended way to handle nullable values in the OpenAPI spec.

@markerikson
Copy link
Collaborator

I believe this is handled by the upstream https://github.com/oazapfts/oazapfts library

@mattoni
Copy link
Author

mattoni commented Oct 20, 2023

Thanks, I've opened an issue there! It'd be nice to support at least the basics of 3.1.0 since it's been out for a few years now.

@mattoni
Copy link
Author

mattoni commented Feb 12, 2024

@markerikson it looks like the upstream version has been updated to include this fix. Does the package version need to be bumped in RTK?

@markerikson
Copy link
Collaborator

@mattoni we can bump the version on our side, but assuming it's a minor release upstream, there's technically nothing we need to do here for people to take advantage of it. You can always regenerate your own lockfile if needed to pick up the latest version of a transitive dependency.

@SimonEggert
Copy link

SimonEggert commented Feb 13, 2024

@markerikson Unfortunately there's a major update involved because the fix has only been applied to current main, so quite some exports of the package have changed or are not available anymore. I think we might need to

  1. Reexport things that this repo relies on on oazapft's side
  2. Update the imports here to match the new paths

I will try to figure out what's missing to make the generator work with the new version. My first impression was that the only thing missing completely in their exports was the ApiGenerator class.

@markerikson
Copy link
Collaborator

@SimonEggert sorry, clarify what new version of oazapfts has the fix?

Looking at https://github.com/oazapfts/oazapfts/releases , I do see that they're working on betas for a v6 major, but I'm not clear what relates to this issue specifically.

@SimonEggert
Copy link

@markerikson Seems that there's a lot going on in that repo indeed 😉

The release that has the fix is 5.1.7 which is a patch release in itself, but RTK is still using 4.8.0. In order to upgrade and include the bugfix we would need to adapt some breaking changes.

@SimonEggert
Copy link

FTR: I opened another issue in the upstream library asking about what the proper way for updating would look like: oazapfts/oazapfts#577

@SimonEggert SimonEggert linked a pull request Feb 14, 2024 that will close this issue
@SimonEggert
Copy link

@markerikson I opened PR #4198 including the necessary oazapfts update if you'd like to take a look.

@mattoni
Copy link
Author

mattoni commented Apr 1, 2024

@markerikson is there anything blocking the above from being merged? Would be great for us to be on the latest so we can support 3.1.0 versioned specs.

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 a pull request may close this issue.

3 participants