reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.64k stars 1.15k forks source link

RTK Query code generation - OpenAPI readonly model properties #2742

Open skupr opened 1 year ago

skupr commented 1 year ago

Please add support for OpenAPI readonly model properties to RTK Query code generation.

I'll try to explain it relatively shortly: For example, I have a simple API with the User object schema, and there's a property id that is set as readOnly: true This allows using the same schema for POST request body and response body as well as and GET request body, for example

POST /user
{ "name": "John"}

returns:

{ "id": 1, "name: John" }

And it also allows using the same schema as response type for the request

GET /user/1

But for that kind of schema, RTK Query code generator just ignores readOnly descriptor and creates

export type User = {
  id: number
  name: string
}

And this basically makes this endpoint

createUser: build.mutation<User, User>({...})

unusuable properly - we have to pass some id

I wonder if RTK Query could handle this and make readOnly properties optional?

phryneas commented 1 year ago

Could you explain how that would change if there were a readonly in there?

Even if it were

export type User = {
  readonly id: number
  name: string
}

you would not get around passing an id in here.

And on runtime level, every of those properties will be readonly anyways.

skupr commented 1 year ago

@phryneas Sorry, I should've explain what's expected.

I see two options:

  1. Simply generate
    export type User = {
    id?: number
    name: string
    }
  2. Generate two different types for request and response:
    
    export type CreateUserRequest = {
    name: string
    }

export type User = { id: string name: string }


(naming is just arbitrary here)
skupr commented 1 year ago

Typescript readonly is basically a different thing. Here's a description of readonly in OpenAPI specs:

readOnly: boolean
Relevant only for Schema "properties" definitions. Declares the property as "read only". This means that it MAY be sent as part of a response but SHOULD NOT be sent as part of the request. If the property is marked as readOnly being true and is in the required list, the required will take effect on the response only. A property MUST NOT be marked as both readOnly and writeOnly being true. Default value is false.

skupr commented 1 year ago

So generating two types seems to me like a more "proper" design according to OpenAPI spces, but it also may be harder to implment.

phryneas commented 1 year ago

I think the "two types" part is probably out of the scope of the codegen, since we only use oazapfts for that generation and don't touch individually properties 🤔

Generally though this reads more like these should just be optional, not readonly - at least from our perspective.

phryneas commented 1 year ago

I mean, I get that this is a special OpenAPI thing, but nothing we can really implement on our level - TypeScript just doesn't know that concept and Oazapfts will always give us one type, not two.

skupr commented 1 year ago

Generally though this reads more like these should just be optional, not readonly - at least from our perspective.

Yes, that's right, it's more like optional on RTK Query side.

dejankortechs commented 1 year ago

Hi @phryneas, we are experiencing the same difficulties with our generated code. Is there any workaround that you have used to resolve this issue temporarily?

cvereterra commented 1 year ago

Hi, I’ve just opened a pull request in oazapfts to support readOnly and writeOnly https://github.com/oazapfts/oazapfts/pull/387

cvereterra commented 1 year ago

https://github.com/oazapfts/oazapfts/commit/57a4e01df039164dbdcb8c5ee25663c2b98f74da added support for this in the 4.7.0 release, I will open a PR to support this OpenAPI feature as soon as I can