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

Generates invalid types when API paths are nested #1752

Open
1 of 2 tasks
mat-certn opened this issue Jul 8, 2024 · 9 comments
Open
1 of 2 tasks

Generates invalid types when API paths are nested #1752

mat-certn opened this issue Jul 8, 2024 · 9 comments
Assignees
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library PRs welcome PRs are welcome to solve this issue!

Comments

@mat-certn
Copy link

Description

We have multiple paths in our schema that are "nested" below each other, like so:

/some/path/{id}
/some/path/{id}/summary

openapi-typescript using the argument --path-params-as-types generates the following:

  [path: `/some/path/${string}`]: SchemaOne;
  [path: `/some/path/${string}/summary`]: SchemaTwo;

which conflicts with the error

'x' index type 'SchemaOne' is not assignable to 'x' index type 'SchemaTwo'.

this is because technically /some/path/${string}/summary is generated as incorrectly assignable to /some/path/${string} - instead of ${string} it should be some sort of type that cannot include "/"

Name Version
openapi-typescript 7.0.2
TypeScript 5.3.3 or 5.5.3 (have tried a few versions)
Node.js 18.18.2
OS + version macOS 14.5

Reproduction

Generate types using the following schema:

openapi: 3.0.3
servers:
  - url: http://some-server.com
    description: Local development server
info:
  title: Example API with conflicting paths
  version: 0.0.1
  license:
    name: MIT
    url: https://opensource.org/licenses/MIT
paths:
  /api/internal/cases/{short_id}:
    get:
      summary: "Get case details"
      operationId: cases_retrieve
      parameters:
        - in: path
          name: short_id
          schema:
            type: string
          required: true
      tags:
        - cases
      security:
        - AuthZeroUserAuth: []
      responses:
        "200":
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/CaseDetail"
          description: ""
        "404":
          description: No response body
  /api/internal/cases/{short_id}/summary:
    get:
      summary: "Get case summary"
      operationId: cases_summary_retrieve
      parameters:
        - in: path
          name: short_id
          schema:
            type: string
          required: true
      tags:
        - cases
      security:
        - AuthZeroUserAuth: []
      responses:
        "200":
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/CaseSummary"
          description: ""
        "404":
          description: No response body
components:
  schemas:
    CaseDetail:
      type: object
      properties:
        case_name:
          type: string
          format: uuid
          readOnly: true
      required:
        - case_name
    CaseSummary:
      type: object
      properties:
        case_summary:
          type: string
          format: uuid
          readOnly: true
      required:
        - case_summary
  securitySchemes:
    AuthZeroUserAuth:
      type: http
      scheme: bearer
      bearerFormat: JWT

npx openapi-typescript example.yml --output example.ts --path-params-as-types

Expected result

A more complex type than ${string} that prevents both endpoints from matching each other.

Checklist

❯ npx @redocly/cli lint example.yml
No configurations were provided -- using built in recommended configuration by default.

validating example.yml...
example.yml: validated in 12ms

Woohoo! Your API description is valid. 🎉
@mat-certn mat-certn added bug Something isn't working openapi-ts Relevant to the openapi-typescript library labels Jul 8, 2024
@phk422
Copy link
Contributor

phk422 commented Jul 8, 2024

Here's one scenario I can think of:

type BasePath = string & { __id: 'BasePath' };

type SchemaOne = {
  case_name: string;
};

type SchemaTwo = {
  case_summary: string;
};

type Paths = {
  [path: `/api/internal/cases/${BasePath}`]: SchemaOne;
  [path: `/api/internal/cases/${BasePath}/summary`]: SchemaTwo;
};

const caseDetail: Paths[`/api/internal/cases/${BasePath}`] = {
  case_name: 'example-case',
};

const caseSummary: Paths[`/api/internal/cases/${BasePath}/summary`] = {
  case_summary: 'example-summary',
};

@drwpow @mat-certn I want to know what you think?

@mat-certn
Copy link
Author

i think for my use-case I want to find the type definition from the URL, e.g. I want to be able to do this:

const url = `/api/internal/cases/12345/summary`

const caseSummary: Paths[url] = {
  case_summary: 'example-summary',
};

which doesn't work with the above: /api/internal/cases/12345/summary does not exist on type Paths

@phk422
Copy link
Contributor

phk422 commented Jul 17, 2024

i think for my use-case I want to find the type definition from the URL, e.g. I want to be able to do this:

const url = `/api/internal/cases/12345/summary`

const caseSummary: Paths[url] = {
  case_summary: 'example-summary',
};

which doesn't work with the above: /api/internal/cases/12345/summary does not exist on type Paths

Are there any good solutions for this in ts?

@drwpow
Copy link
Contributor

drwpow commented Aug 14, 2024

Yeah this is a tricky one! I will admit this flag has always had some rough edges for reasons like this. Loose pattern-matching in TS isn’t as robust as runtime equivalents. I’m sure there are ways to solve it, but they may not be straightforward. Either way, would welcome PRs on improving this.

@drwpow drwpow added the PRs welcome PRs are welcome to solve this issue! label Aug 14, 2024
@drwpow drwpow self-assigned this Nov 27, 2024
@walid-mos
Copy link

walid-mos commented Jan 20, 2025

Hello ! I've got the same problem on my side, any restful crud app is unable to work as without --path-params-as-types, but this flag always generates errors.

My solution for this was to separate paths and dynamic path, to keep types without getting interference between static and dynamic paths.

Here's an example :

export type dynamicPaths = {
	[path: `/api/mydata/${string}`]: {
		...
         }
} 

export interface paths extends dynamicPaths {
          '/api/mydata': {
		...
         }
}

I am creating a PR tonight when i finished to comply to CONTRIBUTING.md, hope this will help and not break any existing code.

Thank you

@gzm0
Copy link
Contributor

gzm0 commented Jan 24, 2025

@walid-mos, thanks a lot for contributing.

I'm commenting here on your proposed approach (rather than on #2106) in attempt to keep all relevant bits of the conversation together.

I'm afraid splitting static / dynamic paths will only solve the issue for some use cases. If /api/mydata/ and /api/mydata/{id} are conflicting, then indeed, splitting the paths can work.

However, consider the OP's example:

/some/path/{id}
/some/path/{id}/summary

Both of these will end up in the dynamicPaths bucket. So they'll end up conflicting anyways.

TBH, I'm not sure there is a good solution to this (unless we get something like microsoft/TypeScript#41160).

@walid-mos
Copy link

Hello @gzm0 ! I see your concerns, for my case, this did not conflict, as the problem is really between dynamic and non dynamic paths.

Here is the example i am using myself :

type DynamicPaths = {
	[path: `/api/tenant/${string}`]: {
		...
	}
} & {
	[path: `/api/tenant/${string}/data-sources`]: {
		...
	}
} & {
	[path: `/api/tenant/${string}/data-sources/files`]: {
		...
} & {
	[path: `/api/tenant/${string}/data-sources/${string}`]: {
		...
	}
} & {
	[path: `/api/llm-admin/${string}`]: {
		...
	}
}

export interface paths extends DynamicPaths {
	'/api/tenant': {
		...
	}
} 

As you can see, /api/tenant/${string} and /api/tenant/${string}/data-sources are falling in your case.

@gzm0
Copy link
Contributor

gzm0 commented Jan 24, 2025

Oh, I understand now. This was about generating invalid types, not the usage of the types being invalid.

I think what "fixes" it in your example is the additional intersection type (see on TS playground).

type DynamicPathsTypechecks = {
	[path: `/api/tenant/${string}`]: A
} & {
	[path: `/api/tenant/${string}/data-sources`]: B
};

type DynamicPathsDoesNotTypecheck = {
	[path: `/api/tenant/${string}`]: A
	[path: `/api/tenant/${string}/data-sources`]: B
};

I think splitting off the static paths is a red herring.

So, we could chose to do this, but IIUC it would simply push the problem later when we actually try to use these types:

// Type usage is still broken.
const x: A = { a: 1 }

// This assignment fails: Type 'A' is not assignable to type 'A & B'.
const y: DynamicPathsTypechecks["/api/tenant/1/data-sources"] = x

It's not entirely clear to me if using intersection types will make overall usability better or worse :-/

@walid-mos
Copy link

Ok i see your point, thank you for the explanation, i was mislead by the fact that every types are the same between paths (of type :

parameters: {
			query?: never
			header?: never
			path?: never
			cookie?: never
		}
		get: {
			parameters: {
				query?: never
				header?: never
				path?: never
				cookie?: never
			}
			requestBody?: never
			responses: {
				/** @description Default Response */
				200: {
					headers: {
						[name: string]: unknown
					}
					content?: never
				}
			}
		}
		put?: never
		post?: never
		delete?: never
		options?: never
		head?: never
		patch?: never
		trace?: never

)

So as i was infering values inside paths interface (like this :

export type ResponseType<T extends RouteData> = T extends {
	responses: {
		[K in IntRange<200, 300>]?: {
			content: { 'application/json': infer Response }
		}
	}
}
	? NonNullable<Response>
	: never

So i was working as it's the same structure, but it's not a valable global solution as it could maybe lead to errors.

Thank you for your time !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working openapi-ts Relevant to the openapi-typescript library PRs welcome PRs are welcome to solve this issue!
Projects
Development

No branches or pull requests

5 participants