7nohe / openapi-react-query-codegen

Node.js library that generates React Query (also called TanStack Query) hooks based on an OpenAPI specification file.
189 stars 18 forks source link

Incorrect implementation of `readOnly` and `writeOnly` #119

Open dan-cooke opened 1 month ago

dan-cooke commented 1 month ago

Thanks for the great lib, I've been looking for something to replace https://github.com/ferdikoomen/openapi-typescript-codegen as its no longer mantained.

There is quite a big issue on that old project too which was never fixed https://github.com/ferdikoomen/openapi-typescript-codegen/issues/432

There have been considerations to fix it in a fork => https://github.com/hey-api/openapi-ts/issues/28

But as of right now, they don't support react query codegen, so i'm out of options.

This library seems well mantained, and I like the API. So I would like to contribute to fixing this issue.

Describe the bug As per the OpenAPI spec for readOnly and writeOnly

You can use the readOnly and writeOnly keywords to mark specific properties as read-only or write-only. This is useful, for example, when GET returns more properties than used in POST – you can use the same schema in both GET and POST and mark the extra properties as readOnly. readOnly properties are included in responses but not in requests, and writeOnly properties may be sent in requests but not in responses.

This implies that when a property is marked readOnly it should not be documented as part of the request object

To Reproduce

  1. Generate the following spec

OpenAPI spec file

openapi: 3.0.3
info:
  title: Templi API
  version: 1.0.0
  description: The REST API for the Templi application.
paths:
  /documents/:
    get:
      operationId: documents_list
      description: API for document CRUD actions
      parameters:
      - name: page
        required: false
        in: query
        description: A page number within the paginated result set.
        schema:
          type: integer
      tags:
      - documents
      security:
      - auth0: []
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Document'
openapi: 3.0.3
info:
  title: Templi API
  version: 1.0.0
  description: The REST API for the Templi application.
paths:
  /documents/:
    get:
      operationId: documents_list
      description: API for document CRUD actions
      parameters:
      - name: page
        required: false
        in: query
        description: A page number within the paginated result set.
        schema:
          type: integer
      tags:
      - documents
      security:
      - auth0: []
      responses:
        '200':
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/PaginatedDocumentList'
          description: ''
components:
  schemas:
    Document:
      type: object
      properties:
        creator:
          allOf:
          - $ref: '#/components/schemas/Auth0User'
          readOnly: true
        id:
          type: integer
          readOnly: true
        orgId:
          type: integer
          writeOnly: true
      required:
      - name

Expected behavior openapi/requests/types.gen.ts should make a differentation between a requset and response schema

ie.

// one type for requesting with readOnly props removed
export type CreateDocumentRequest = {
    name: string;
    orgId: number
};

// one type for response with writeOnly props removed
export type Document = {
    id: number
    name: string;
}

Actual behaviour The ts readonly keyword is just prepended to fields with readonly, which is not semantically correct by OpenAPI spec

export type Document = {
    readonly id: number;
    name: string;
    orgId: number
};
dan-cooke commented 1 month ago

Another easy solution with minimal change would be to follow the advice on this comment

If you wrapped the create/update hook requestBody types with this:

export type OmitReadonly<T extends object> = Omit<T, ReadonlyKeys<T>>

That would fix the ReadOnly part of this anyway..

Write only is a bit more involved so I believe having two separate type definitions - one for requesting, and one for responding is the best way

seriouslag commented 2 weeks ago

This library uses @hey-api/openapi-ts to generate the typescript models and service layer. The effort to fix that package would be better for the ecosystem IMO.