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

Better Type Resolution #1673

Open
4 tasks done
blipk opened this issue Sep 6, 2024 · 14 comments
Open
4 tasks done

Better Type Resolution #1673

blipk opened this issue Sep 6, 2024 · 14 comments

Comments

@blipk
Copy link

blipk commented Sep 6, 2024

Sorting

  • I'm submitting a ...

    • bug report
    • feature request
    • support request
  • I confirm that I

    • used the search to make sure that a similar issue hasn't already been submit

Behavior

There are multiple issues regarding type resolution (i.e. types to schemas) and use of generics in tsoa.

I suggest other issues be closed and merged into this issue and the type resolution system improved.

The largest issue I have is with resolving intersection types #1067

The second largest issue is with resolution of wrapped types - tsoa seems to want to add them all to my schema but my controllers only return the top level type and I would prefer schemas just be generated for that:

e.g. Gives the error GenerateMetadataError: No matching model found for referenced type M.

type UserReadResponse =
    ReadResponseBody<UserModelDTO, "User", "password">
type OmitKeysTypes = string | number | symbol

type ReadResponseContentOmittedKeysT = "deletedAt"
type ReadResponseContent<M, OmitKeys extends OmitKeysTypes = never> =
    Omit<M, OmitKeys | ReadResponseContentOmittedKeysT>

type ReadResponseBody<M, MTypeName extends string, OmitKeys extends OmitKeysTypes = never> =
    DataResponseBody<ReadResponseContent<M, OmitKeys>, MTypeName, FoundResponseMessageT>

If I explicitly only type the UserReadResponse then tsoa is fine with it - but I would prefer the generics as they're easier to use across multiple controllers

e.g.

// This works
type UserReadResponse =
   DataResponseBody<Omit<UserModelDTO, "password" | ReadResponseContentOmittedKeysT>, "User", FoundResponseMessageT>

// This also works - so I don't see why the full example should not
type UserReadResponse =
    DataResponseBody<ReadResponseContent<UserModelDTO, UserReadResponseOmittedKeysT>, "User", FoundResponseMessageT>

Perhaps TSOA should provide a utility type that marks the sub-types to be ignored in schema generation.

Maybe there's another way to declare these types so TSOA could resolve them fine - I would appreciate any hints on that until a better solution can be merged.

Related Issues

Typing to schemas
#1067
#1559
#1327
#1267
#1268
#1547

Generics usage:
#1665

Possibly related:
#705
#1021

I might be willing to help with a PR for this - if @lukeautry or @WoH could let me know where a good place to start would be.

@douglasg14b
Copy link

douglasg14b commented Sep 7, 2024

Collecting these all in one place is extremely valuable.

Question: Is the way TSOA generates/resolves these replicateable by any other libraries in 2024? Essentially, can TSOA make use of a more robust type interpretation tool instead of building it on it's own?


Example 1:

    @Post()
    testEndpoint<TMethod extends 'one' | 'two'>(@Path() method: TMethod) {
        return method;
    }

No matching model found for referenced type TMethod

This is a contrived example, however such usage of generics becomes insanely valuable (required) when safely resolving mapped types directly from action inputs. For example, if TMethod is used to index MethodArgs<TMethod>. As this would define the expected data given a particular path (Essentially semi-dynamic endpoint construction in the rare cases where such a thing is desirable)

Example 2 #1622

  • There is some really good information in here on how we might get started on debugging & fixing this. I just need some time & energy to dedicate to this for like a week (or more, this codebase takes extra work to grep & grok) to onboard to TSOAs internal, but alas this has yet to happen :(

@blipk
Copy link
Author

blipk commented Sep 7, 2024

I think the problem is tsoa tries to generate schemas for all the intermediate types, but obviously a schema cant be made when their generic parameters don't have values, only at the top level where the intermediate type/s are used do the values of the generics come in.

I think tsoa should either (although I'm no expert in this):
a) Ignore all the types it can't resolve and just make a schema for the top level type
b) Offer a/some utility types that either mark the top level type, or the unresolveable intermediate types

Before that though, I think fixing the linked issue with intersection types should be a lot easier and pertinent

Another issue that might be easy is to resolve TypeQuery types and other inferences, when they map to concrete types - e.g. right now tsoa complains if I use a type MyStringT = typeof "My String" which should be concrete enough to be able to generate schemas from

@blipk
Copy link
Author

blipk commented Sep 7, 2024

GPT Helped me write this conditional type so I can transform my types without intersections - although it doesnt really solve the issue it works well in some cases.

type TransformKeysNoIntersections<T, OmitKeys extends keyof T, OptionalKeys extends keyof T, RequiredKeys extends keyof T> = Omit<{
        [P in keyof T]: P extends OptionalKeys
            ? T[P] | undefined // Mark as optional
            : P extends RequiredKeys
            ? T[P] // Keep as required
            : T[P]; // Keep as is
    }, OmitKeys>;

I actually get the below warning but I can see in swagger that the schema and example are exactly how I specified - with the keys requested made optional, required, or omitted.

Warning: This kind (200) is unhandled, so the type will be any, and no type conflict checks will made
At:  /app/src/responders/controllerResponders.ts:207:207.
This was caused by 'Omit<{
        [P in keyof T]: P extends OptionalKeys
            ? T[P] | undefined // Mark as optional
            : P extends RequiredKeys
            ? T[P] // Keep as required
            : T[P]; // Keep as is
    }, OmitKeys>'

There's also this for simpler intersections but both of these utilities suffer from the other issue if theyre used more than a couple levels deep in the typing chain

type MergeTypes<T1, T2> = {
    [K in keyof T1 | keyof T2]: K extends keyof T2
      ? T2[K]
      : K extends keyof T1
      ? T1[K]
      : never;
};

@WoH
Copy link
Collaborator

WoH commented Sep 7, 2024

Collecting these all in one place is extremely valuable.

Question: Is the way TSOA generates/resolves these replicateable by any other libraries in 2024? Essentially, can TSOA make use of a more robust type interpretation tool instead of building it on it's own?

Yes and no. Imo you're correct that rebuilding the Specs from the Typescript AST was viable in a world of simpler types, however, the path forward would definitely be to leverage the TS type checker more. There's some initial work that we never got over the finish line.

@douglasg14b
Copy link

douglasg14b commented Sep 7, 2024

however, the path forward would definitely be to leverage the TS type checker more

Can you expand on this? Does the 1st party TS ecosystem provide utility today that it did not a few years ago that simplifies this problem? If so, what tools/utilities/programs/APIs are on your mind?

Edit: The type checker APIs mentioned here? https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#type-checker-apis

Edit2: Ohhh, the type checker API may actually be it, it seems deceptively straight forward 🤔.

Edit3: Found this, but that might not be it: https://github.com/dsherret/ts-morph

Edit4: Looks like this lib makes use of the type checker for an implementation example: https://github.com/ts-oas/ts-oas/blob/master/src/lib/SchemaGenerator.ts skimming through it the implementation appears much easier to grok, but that may be up to style difference not implementation differences.

@douglasg14b
Copy link

Is there an intermediary language that can be utilized here to simplify the output needs? Or is that the AST?

I've found in other projects where I'm trying to transform one language, data structure, or something else for multiple output use cases, that picking an intermediary language to work with simplifies and decouples the implementation. If anything, it tends to make it easier to reason about.

@blipk
Copy link
Author

blipk commented Sep 7, 2024

@douglasg14b I've looked through some of the tsoa code - a lot of it happens here and here and the adjacent files.

Theyre using the same typescript APIs as what you linked and you can see some attempts at updating it - would probably be best to use the typescript APIs directly instead of wrapper libraries, although they could be a good reference.

ts-oas looks interesting and a lot closer to what I preferred when using FastAPI - but it only generates the spec and doesnt integrate with controllers/routes like tsoa does

The FastAPI integration with Pydantic is extremely good and I suggest that tsoa takes as much inspiration from it as possible - tighter integration with a specific validation library like class-validator would be great

I also think it would be better to provide a lot of metadata via decorators rather than jsdoc comments - and also allow choosing a name for the generated schema items e.g.

@Model("MyModelName", "Description of MyModel", { example: "of my model" })

The type resolver issues should be a priority though before any of that is considered.

@vmarchaud
Copy link

FYI we previously used tsoa and @Eywek wrote typoa and oatyp that use ts-morph instead of re-implementing the type logic resolution and it worked great for us if you want to check it out

@blipk
Copy link
Author

blipk commented Sep 12, 2024

@vmarchaud If it works as good as it claims in comparison to tsoa - I wish I had known earlier.

tsoa caused me so many headaches and wasted time with its type resolution

the API looks similar though - it might have been nicer to contribute to tsoa instead.

Maybe I will try it and see how it goes. Do you think it would be easy enough to swap out tsoa for typoa? Does it support all the same decorators?

EDIT: Looks like it was forked a few years ago - missing a few of the decorators I'm using e.g. @Middleware

@blipk
Copy link
Author

blipk commented Sep 12, 2024

@WoH considering @vmarchaud comment - it might be good to consider ts-morph

@WoH
Copy link
Collaborator

WoH commented Sep 13, 2024

@WoH considering @vmarchaud comment - it might be good to consider ts-morph

Feel free to open a PR!

@vmarchaud
Copy link

the API looks similar though - it might have been nicer to contribute to tsoa instead.

tsoa isn't the same implementation as typoa, i don't think it would have make sense to open a PR that refactor the whole projet down to what typoa is and call it a day.

Maybe I will try it and see how it goes. Do you think it would be easy enough to swap out tsoa for typoa? Does it support all the same decorators?
EDIT: Looks like it was forked a few years ago - missing a few of the decorators I'm using e.g. @Middleware

It might not contain all features that are supported by tsoa (since we wrote it for our usage) but adding middleware shouldn't be that difficult, the codebase is quite small.
I'm not offering a drop-in replacement (and we have no interested in doing so currently), however since you looked around for a solution to your problem, i thought it might be interesting to you that the ts-morph path is totally viable, whatever it would be upstreaming it in tsoa or using typoa directly.

@blipk
Copy link
Author

blipk commented Sep 15, 2024

I started using the ts type checker in a personal project and had a look at ts-morph - it does provide some decent utilities and abstractions - but it seems like it's just adding another level of complexity on top of the already complex typescript compiler API.

If I want to understand the ts-morph abstractions I really should understand the compiler API anyway - so I'm not sure its a good idea to use it.

Considering the complexity of tsoa already - I think it would be better to stick with its internal wrapper over the API.

In saying that @WoH

Can you offer any development notes? Root package.json does nothing and tests have failures.

I got the packages subdirs to build - but I don't have any experience working on monorepos like this.

Can I install the packages locally and use them in other local projects? Or is any recommendations/notes on debugging/testing?

@WoH
Copy link
Collaborator

WoH commented Sep 15, 2024

I'm not sure where the test fails come from, but you can take a look at the CI, which is the reference point for these, if they wouldn't run there, we'd know.

tsoa is 2 packages, the cli, which contains the code -> compiler/type resolver -> metadata -> template & openapi pipeline and the runtime, which contains a few helpers for the code generated via the templates.

There should be a vscode test debug config that you can use, we have unit tests Ind integration tests, we will usually always run the cli over a set of fixtures and then perform assertions on them, at least in the code that this would target.

The type resolver is the heart of the compiler part.

E: Possibly this is what you're also seeing?
#1677 (comment)

@github-actions github-actions bot added Stale and removed Stale labels Oct 16, 2024
Repository owner deleted a comment from blipk Oct 31, 2024
Repository owner deleted a comment from github-actions bot Oct 31, 2024
Repository owner deleted a comment from douglasg14b Oct 31, 2024
Repository owner deleted a comment from douglasg14b Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants