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

Refactor type aliases #444

Closed
wants to merge 2 commits into from
Closed

Refactor type aliases #444

wants to merge 2 commits into from

Conversation

WoH
Copy link
Collaborator

@WoH WoH commented Aug 23, 2019

Historic context:
As far as I can tell, resolving type aliases was introduced to support a narrow set of intersection types. 2 interfaces were scanned for properties and merged together.

Why may this not make sense anymore?
Now that we can properly represent the metadata of an intersection, we can use that to support the (narrowly defined) Swagger 2 representation of 2 interfaces.
However, we should eliminate that code path from type resolving and handle type aliases "properly": #429

What's the cost?
This would break some swagger component/definition names (In my mind, the new representation is better and consistent with nested interfaces, that's why I would prefer the new way):

#/components/schemas/Namespace_InnerNamespace_TestModel
would become
#/components/schemas/Namespace.InnerNamespace.TestModel

(OpenAPI3, Swagger is similar)

Furthermore, there would no longer be a definition/component for the alias, the content would be inlined.

What's the benefit?
We remove a legacy code path from the typeResolver that does not make sense anymore.
We get more consistent naming with fewer code.
We avoid issues with type aliases themselves (we forward the resolution responsibility to the thing referenced by the type alias)
We are closer to the TypeScript internals (See #429) and it would make it easier to support i.e. TS convenience types.
Why?: Pick, Omit, ..., all the convenience types are type aliases.
If we want to support them at a point in time, the way to do that would be to resolve the mapped type py passing the arguments to the type alias to the mapped type.

type Pick<T, K extends keyof T> = {
    [P in K]: T[P];
};

@HoldYourWaffle
Copy link
Contributor

Since I'm currently cleaning up the type resolver a bit (#445), we probably shouldn't merge this until that's merged to avoid conflicts.
(That is assuming the maintainers want my cleanup merged)

@WoH
Copy link
Collaborator Author

WoH commented Sep 23, 2019

Closing this in favor of another PR with a more advanced implementation based on #429 (comment) (currently blocked by #470)

@WoH WoH closed this Sep 23, 2019
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

Successfully merging this pull request may close these issues.

2 participants