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

Enhanced types reflection system #296

Open
19majkel94 opened this issue Mar 30, 2019 · 12 comments

Comments

@19majkel94
Copy link
Owner

commented Mar 30, 2019

Introduction words

TypeGraphQL was created mainly to reduce the duplication in declaring GraphQL schema types and TypeScript types by synchronizing them in one source of truth using decorators and reflection magic.

Problem description

Probably the most annoying thing in TypeGraphQL is still the types duplication:

@Field(type => [String])  // <- here we have the TypeGraphQL type hint
propertyName: string[];   // <- and here we have the TS type

The even worse thing is that this types can be silently mismatched:

class Test {
  @Field(() => [Author])   // <- wrong item type
  actors: Actor[];

  @Field()                 // <- required in schema
  optionalField?: string;  // <- optional in TS
}

Typed decorators (#221) are unfortunately only a partial solution for providing enhanced type-safety.

The root cause

The main cause of this situation is the limited limited reflection capabilities of TypeScript emitDecoratorMetadata option - it doesn't work with generics (string[], Array<number>, Promise<Recipe>) and it's not even able to detect optional/nullable fields (?: string, : number | null).

In the last few weeks I was working on a TypeScript compiler plugin that can enhance the built-in, limited reflection capabilities. The goal is to remove the need of providing the type => [String] hints by emitting full type info about properties type and methods return type.

Solution for the problem

The Proof of Concept is available in the separate repository:
https://github.com/19majkel94/typegraphql-reflection-poc

For now it can read and emit basic type info - primitives (like boolean, string), nullable types (?: string, : number | null), arrays (string[], Array<number>) and promises (Promise<Recipe>) as well as their combinations like Promise<Array<string | null> | null>.

So just by declaring TypeScript types:

@ObjectType
export class Sample {
  @Field
  dateField!: Date;

  @Field
  optionalStringField?: string;

  @Field
  nullableStringArrayField!: Array<string | null>;

  @Field
  nullableStringNullableArrayPromiseField!: Promise<Array<string | null> | null>;
}

It's possible to read their type info in runtime and generate this GraphQL schema type:

type Sample {
  dateField: Date!
  optionalStringField: String
  nullableStringArrayField: [String]!
  nullableStringNullableArrayPromiseField: [String]
}

The only drawback is that you have to use an alternative compiler, like ttypescript to run the transform plugin and emit enhanced type metadata. But as it integrates well with ts-node (ts-node -C ttypescript src/example/index.ts), I think that it's not so bad solution.

Future work

That proof of concept supports only basic subset of types. There is still a lot of work to be done (i.a. detecting args in methods, inline unions, generic types support) - you can find more info here.

If you love this idea but you can't help with the code by making some contributions, please consider supporting the project and our development efforts! ❤️

@19majkel94 19majkel94 added this to the 2.0.0 release milestone Mar 30, 2019

@19majkel94 19majkel94 self-assigned this Mar 30, 2019

@19majkel94 19majkel94 added this to Backlog in Board via automation Mar 30, 2019

@19majkel94 19majkel94 pinned this issue Mar 30, 2019

@Veetaha

This comment has been minimized.

Copy link

commented Mar 30, 2019

I wish there was some library (decoupled from typegraphql) that would emit all this type information for runtime in a simple, well-defined, uniform way. Did I get you right, that you intend to make it?

@19majkel94

This comment has been minimized.

Copy link
Owner Author

commented Mar 30, 2019

@Veetaha For now I will only focus on TypeGraphQL use case, maybe even with pre-compiled type info (like nullable: "itemsAndList").

Later I can extend the support for other use cases, with the ability to consume the type metadata by other users or libraries. But this would need a wider discussion with the TS community about defining the API in "a simple, well-defined, uniform way" 😉

@Veetaha

This comment has been minimized.

Copy link

commented Mar 30, 2019

@19majkel94 By the way, are you going to support matrix-like types (jagged arrays)? "itemsAndList" is not that much scalable in my point of view.

@19majkel94

This comment has been minimized.

Copy link
Owner Author

commented Mar 30, 2019

are you going to support matrix-like types (jagged arrays)?

See #277 and FAQ:
https://typegraphql.ml/docs/faq.html#how-can-i-define-the-two-dimension-array-nested-arrays-array-of-arrays

nullableStringNullableArrayField!: Array<string | null> | null;
nullableStringNullableArrayField: [String]

That's an example of nullable: "itemsAndList", just like you provide it now in decorator options:
https://typegraphql.ml/docs/types-and-fields.html#docsNav

if we need a sparse array, we can control the list items nullability via nullable: "items" (for [Item]!) or nullable: "itemsAndList" (for the [Item]) option.

@Veetaha

This comment has been minimized.

Copy link

commented Mar 30, 2019

Oh, yeah, right, shame on me 😅

@Veetaha

This comment has been minimized.

Copy link

commented Mar 30, 2019

@19majkel94 By the way, I'd rather use an enum instead of raw string literals, like

export const enum Nullability {
    Absent   = 0b00,
    ForValue = 0b01,
    ForItems = 0b10,
    Deep     = ForValue | ForItems
}

export function isNullableValue(nullity: Nullability) {
    return Boolean(nullity & Nullability.ForValue);
}

export function hasNullableItems(nullity: Nullability) {
    return Boolean(nullity & Nullability.ForItems);
}
// ...
Comparing numbers instead of strings might give a slight performance boost
@j

This comment has been minimized.

Copy link

commented Apr 4, 2019

@19majkel94 I haven't looked into ttypescript, but does the compiler support decorators on interfaces?

It would be so nice to pass vanilla objects and use field resolvers to do "magic" work instead of class instances. I just haven't looked into ttypescript, but it looks cool.

@19majkel94

This comment has been minimized.

Copy link
Owner Author

commented Apr 4, 2019

@j
ttypescript is just a TypeScript CLI that support custom transformers. Raw typescript supports it programmatically, so all webpack ts-loader and other TS compiler already supports the plugins.

support decorators on interfaces

Interfaces doesn't exist in runtime, so there's no runtime value that you can reference like classes. Maybe in the distant future I will support interfaces with annotating fields in JSDoc comments.

And to support decorators, you would have to fork TypeScript to remove all the check and errors that other parts of language services and checker might produce.

@anodynos

This comment has been minimized.

Copy link

commented Apr 7, 2019

This looks so promising - awesome work @19majkel94

@schickling

This comment has been minimized.

Copy link

commented Apr 15, 2019

Great initiative @19majkel94! Can you elaborate how this will work with VSCode which has TS support built-in?

@19majkel94

This comment has been minimized.

Copy link
Owner Author

commented Apr 15, 2019

@schickling
There's no changes on the language server side - the transformer plugin is purely for emitting metadata in the builded JS code so that they can be read later in runtime.

So the only change for users is removing the need of typing () => [Author]. All TS features like typechecking, renaming, etc. works the same.

@Veetaha

This comment has been minimized.

Copy link

commented Apr 15, 2019

This whole idea seems great, maybe there already exist some projects employing this feature we could learn from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.