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

Readonly Type Generation Fails TypeChecking with Single-Files #823

Closed
DerekRies opened this issue Apr 11, 2023 · 3 comments
Closed

Readonly Type Generation Fails TypeChecking with Single-Files #823

DerekRies opened this issue Apr 11, 2023 · 3 comments

Comments

@DerekRies
Copy link

What are the steps to reproduce this issue?

With the following OpenAPI spec:

openapi: 3.1.0
info:
  title: Readonly CodeGen Test
  version: 1.0.0
paths:
  "/people":
    post:
      operationId: createPerson
      description: Create a new Person
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/Person"
      responses:
        200:
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Person"
components:
  schemas:
    Person:
      type: object
      properties:
        id:
          type: number
          readOnly: true
        first_name:
          type: string
          description: The first name of the person
        last_name:
          type: string
          description: The last name of the person
      required:
        - id
        - first_name
        - last_name

And the following config

module.exports = {
  readonly: {
    input: "./openapi.yml",
    output: "./output.ts",
  },
};

What happens?

The following code is generated:

/**
 * Generated by orval v6.14.0 🍺
 * Do not edit manually.
 * Readonly CodeGen Test
 * OpenAPI spec version: 1.0.0
 */
import axios from "axios";
import type { AxiosRequestConfig, AxiosResponse } from "axios";
export interface Person {
  readonly id: number;
  /** The first name of the person */
  first_name: string;
  /** The last name of the person */
  last_name: string;
}

/**
 * Create a new Person
 */
export const createPerson = <TData = AxiosResponse<Person>>(
  person: NonReadonly<Person>, // Error: Cannot find name 'NonReadonly'. Did you mean 'Readonly'? ts(2552)
  options?: AxiosRequestConfig
): Promise<TData> => {
  return axios.post(`/people`, person, options);
};

export type CreatePersonResult = AxiosResponse<Person>;

The code has type errors right out of the gate. It looks like when #813 introduced the read-only functionality that this import of NonReadonly<T> (and the actual definition) only occurs when the models are generated into separate files (and have an index.ts based export).

What were you expecting to happen?

For the NonReadonly<T> type to be defined (or imported from a known location) before being referenced.

Any other comments?

In the working case (splitting models), these utility types appear to be generated with the code. Any reason why they aren't just imported from a utility type package or something?

What versions are you using?

Operating System: macOS 12.6.3
Package Version: [email protected]
Browser Version: n/a

@anymaniax
Copy link
Collaborator

anymaniax commented Apr 11, 2023

fixed with 6.14.1 and I don't want to introduce dependencies in the generated code. It's not the responsibility of Orval to choose a dependency and force you to install something.

@DerekRies
Copy link
Author

Thanks for the quick turnaround!

It's not the responsibility of Orval to choose a dependency and force you to install something.

Fair enough. There's something to be said for output that's totally standalone.

Curious for your take on this slight awkwardness related to these types though, and partially the reason why I was hinting at putting the type utilities into a package like the following

import type { NonReadonly } from "@orval/type-utils";

But anyway, since those Type Utilities aren't exported, consumers of the API client can't use them.

This isn't a problem if you're providing the request payload inline via the generated client for example. But if the consumer has to create the payload before the operation call, there's no easy mechanism for using the actual type.

import { createPerson } from './person';

const resp = await createPerson({ first_name: 'test', last_name: 'test' }); // This is alright, because TypeScript knows the type

const payload = { first_name: 'test', last_name: 'test' }; // Awkward
const resp2 = await createPerson(payload); 

This works, but is awkward because you don't get any tooling helping you with the type at the point of definition, just where its being used as an argument for createPerson.

Options

Omit<T, K> (Consumer Side)

You could do something like

import { createPerson, Person } from './person';

const payload: Omit<Person, 'id'> = { first_name: 'test', last_name: 'test' };
const resp = await createPerson(payload); 

But that's also awkward as the consumer is now responsible for rewriting the types already defined by the contract (i.e. consumer hardcodes which fields should be omitted).

Parameters<T> (Consumer Side)

Or you could do

import { createPerson } from './person';

const payload: Parameters<typeof createPerson>[0] = { first_name: 'test', last_name: 'test' };
const resp = await createPerson(payload); 

Which does exactly what it needs to, but it's not exactly the cleanest or most obvious looking type in the world. This is probably what I'll do when it's needed.

Export the Type Utility (Orval Side)

If the type utility were exported then you could just use it directly

import { createPerson, NonReadonly } from './person';
const payload: NonReadonly<Person> = { first_name: 'test', last_name: 'test' };

This has the potential downside of making it not so easy to just export * from './generated-code' if you're trying to combine a bunch of different OpenAPI specs and generated outputs into a singular export for use somewhere else since the same Identifiers (NonReadOnly, WritableKeys, etc...) can technically be exported from each of those modules now.

Exporting a Type Alias (Orval Side)

Or a separate type could be created for the RequestPayloads and ResponseBodies

export type PersonRequestPayload = NonReadonly<Person>;

There's probably more opportunity for naming collisions if the tool is generating and defining novel types though.

@anymaniax
Copy link
Collaborator

@DerekRies I am maybe wrong on this I am not really sure to be honest. I always saw orval as a tool that’s doesn’t need to be installed in the projet and that can be removed easily if it’s doesn’t fit your need anymore. I am really open to change this behavior if the majority think it’s better to do another way. It’s really difficult to feed the expectation of everyone 😅.

@melloware melloware closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2025
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

3 participants