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

Incorrect implementation of readOnly and writeOnly #119

Open
dan-cooke opened this issue May 21, 2024 · 2 comments
Open

Incorrect implementation of readOnly and writeOnly #119

dan-cooke opened this issue May 21, 2024 · 2 comments

Comments

@dan-cooke
Copy link

dan-cooke commented May 21, 2024

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 ferdikoomen/openapi-typescript-codegen#432

There have been considerations to fix it in a fork => hey-api/openapi-ts#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
};
  • OS: Arch Linux
@dan-cooke
Copy link
Author

dan-cooke commented May 21, 2024

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
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants