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
MichalLytek opened this issue Mar 30, 2019 · 15 comments
Open

Enhanced types reflection system #296

MichalLytek opened this issue Mar 30, 2019 · 15 comments
Assignees
Labels
Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request
Milestone

Comments

@MichalLytek
Copy link
Owner

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! ❤️

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Discussion 💬 Brainstorm about the idea labels Mar 30, 2019
@MichalLytek MichalLytek added this to the 2.0.0 release milestone Mar 30, 2019
@MichalLytek MichalLytek self-assigned this Mar 30, 2019
@MichalLytek MichalLytek pinned this issue Mar 30, 2019
@Veetaha
Copy link

Veetaha 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?

@MichalLytek
Copy link
Owner Author

@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
Copy link

Veetaha 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.

@MichalLytek
Copy link
Owner Author

MichalLytek 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
Copy link

Veetaha commented Mar 30, 2019

Oh, yeah, right, shame on me 😅

@Veetaha
Copy link

Veetaha 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
Copy link
Contributor

j 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.

@MichalLytek
Copy link
Owner Author

@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
Copy link

anodynos commented Apr 7, 2019

This looks so promising - awesome work @19majkel94

@schickling
Copy link

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

@MichalLytek
Copy link
Owner Author

@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
Copy link

Veetaha commented Apr 15, 2019

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

@Superd22
Copy link
Contributor

Superd22 commented Aug 8, 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"

I do think decoupling the typescript plugin and its consumption by type-graphql is the right path to take. We could at first simply focus on the use-case needed for type-graphql, but the plugin would definitely bring attention (and contributions), imho.

I'd be down to help work on that by the way!

@mansoor292
Copy link

I was hunting around and found this issue. Could this plugin infer from existing types for a full graphql schema to write a graphqlify query that can then be translated back to the original type. The resulting object is then of the original type and has null for fields not present in the query?

I am looking for combining the ‘official’ type with the graphqlify type.

I hope I am making sense.

@MentalGear
Copy link

microsoft/TypeScript#3628 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants