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

MetadataStorage: Access to unbuilt metadata #134

Open
adam-stanek opened this issue Aug 26, 2018 · 14 comments
Open

MetadataStorage: Access to unbuilt metadata #134

adam-stanek opened this issue Aug 26, 2018 · 14 comments
Labels
Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request

Comments

@adam-stanek
Copy link

The current MetadataStorage works in two phases.

  1. It gathers all the definitions and stores them in unprocessed arrays
  2. Builds collective metadata from them during the schema generation

The problem is that if I want to access the metadata (ie. defined fields) anytime before the schema generation occurs I cannot do that because it was not built yet. This prevents use-cases like generating dynamic input types / object types based on existing metadata.

Example of the problem:

I have CreateUser input type and I want to create UpdateUser input type which would have exactly the same fields as CreateUser but all nullable (basically GraphQL equivalent to TS Partial<T>).

For this I would like to access metadata definition of CreateUser but I cannot do it directly because the metadata has not been built yet. Instead I have to access the information about input type's fields through access to private property.

Here is a described helper with an workaround for current situation:

import { getMetadataStorage } from 'type-graphql/metadata/getMetadataStorage'
import { InputType } from 'type-graphql'

/**
 * Creates partial sub-type of given InputType.
 *
 * Usage:
 * @InputType()
 * export class UpdateUserDTO extends createPartialInputType(CreateUserDTO) {}
 */
export function createPartialInputType<T>(InputTypeClass: {
  new (...args: any[]): T
}): { new (...args: any[]): Partial<T> } {
  @InputType()
  class PartialInputType extends (InputTypeClass as any) {}

  const fields = (getMetadataStorage() as any).fields
  fields.push(
    ...fields.filter((f: any) => f.target === InputTypeClass).map((f: any) => ({
      ...f,
      target: PartialInputType,
      typeOptions: { ...f.typeOptions, nullable: true },
    })),
  )

  return PartialInputType as any
}

Ideal solution

I think we should rethink the way we store the metadata (at least for input / object types) and register them directly within a built structure of given target. We would have to think carefully about which information is available at given time.

The minimum effort solution

If you think that the redesign of MetadataStorage is not worth the trouble (or will not be within next few versions), it would be nice to add at least this method to the public API of MetadataStorage.

findFields(target: Function): FieldMetadata[] {
   return this.fields.filter(field => field.target === target);
}
@MichalLytek
Copy link
Owner

First of all, accessing internal stuff is strongly prohibited. They are a subject of constant changes and they might be renamed, moved or even removed without any notice. To prevent that kind of problems, I will switch to publishing the package bundled with rollup, so there will be no way to access internal, private stuff. Users can safely interact only with things that are exported in index file.

And yes, I've described the drawbacks of current metadata storage architecture and how I'm going to improve it here: #133 (comment)
After that I can think about a plugin system with a API that other could use to interact with the metadata or schema generation process. This will be the only supported way to alter the TypeGraphQL behavior. Of course you can always fork this repo and do it on your own.

For your CreateUser and UpdateUser case I would recommend using the higher order class pattern and define it explicitly:

export function UserClass(nullable: boolean) {
  @InputType(nullable ? "UpdateUser" : "CreateUser")
  class User {
    @Field({ nullable })
    name: string;

    @Field(type => Int, { nullable })
    age: number;
  }

  return User;
}

export const CreateUser = NullableHOC(UserClass, false);
export const UpdateUser = NullableHOC(UserClass, true);
export type CreateUser = InstanceType<typeof CreateUser>;
export type UpdateUser = InstanceType<typeof UpdateUser>;

This allows you to run this operations:

mutation {
  addUser(data: { name: "name", age: 2 })
  updateUser(name: "name", data: { age: 3 })
}

I should probably better document this feature 🤔
A bunch of helpers to have a good type safety:

export function NullableHOC<T>(
  hoc: (nullable: boolean) => T,
  nullable: true,
): HOCClass<typeof hoc, typeof nullable>;
export function NullableHOC<T>(
  hoc: (nullable: boolean) => T,
  nullable: false,
): HOCClass<typeof hoc, typeof nullable>;
export function NullableHOC<T>(hoc: (nullable: boolean) => T, nullable: boolean) {
  return hoc(nullable);
}

export type InstanceType<T> = T extends ClassType<infer R> ? R : never;

export type HOCClass<T extends Function, U extends boolean> = T extends (...args: any[]) => infer R
  ? U extends true ? ClassType<Partial<InstanceType<R>>> : R
  : never;

@MichalLytek MichalLytek added Question ❔ Not future request, proposal or bug issue Discussion 💬 Brainstorm about the idea labels Aug 26, 2018
@adam-stanek
Copy link
Author

The whole point of my suggestion was to make the metadata API public, not to advocate accessing internals. The mentioned problem / snippet was just to illustrate the point of what I needed to access and why. Your solution to the mentioned problem is elegant and I am thankful for it but it is not something I am trying to discuss here.

I know that you are planning on redesigning the way how the metadata gets handled. Thats why I wanted to suggest other possible feature requirements for you to consider. The mentioned problem is solvable using HoC but it is not a silver bullet. Anything that needs more than parametrizing few values can be messy. For example lets say I would like to create filter input type for it with different operators for each field based on it's type.

I believe that exposing input / object type metadata can be beneficial and I don't see much downside to it. If the API is designed well there is no need to hide it.

@MichalLytek
Copy link
Owner

The whole point of my suggestion was to make the metadata API public,

I agree with that. But it's not the right time for it as it's a subject of constant changes - do you want breaking changes and major releases every week that would force you to change your code relying on metadata? I guess not 😉

Anything that needs more than parametrizing few values can be messy. For example lets say I would like to create filter input type for it with different operators for each field based on it's type.

From my experience, doing such complicated things like you presented with create/update type or filter type with parametrizing few values, just in favor of reducing boilerplate, is only a source of bugs. There's a lot of magic in this dirty tricks, which makes things incomprehensible for other teammates and that becomes a real pain while trying to debug it later when some problems appear.

And I would prefer exposing dedicated tools with nice API for solving problems like:
const UpdateUser = createDerivativeType(CreateUser, ({ metadata }) => { ... });
rather than giving access to internal for creating crazy workarounds.

You are not the first who wants to generate the partial type or needs generic type, so it's better to report it, discuss the possibilities and create a solution that others could use too, rather than solving it by your own.

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community and removed Question ❔ Not future request, proposal or bug issue labels Aug 27, 2018
@MichalLytek MichalLytek added this to the 1.0.0 release milestone Sep 2, 2018
@MichalLytek
Copy link
Owner

I am upholding the decision to not expose the internal stuff.

For now all the use cases of exposing metadata storage is about creating derivative types. So I think it's better to create a dedicated API for that case, rather than digging in internals or use weird hooks and workarounds to achieve the same.

I think we should rethink the way we store the metadata (at least for input / object types) and register them directly within a built structure of given target. We would have to think carefully about which information is available at given time.

This is the core problem - that's why metadata storage has phase. At first we collect just the data from decorators, then we can build the complete metadata structure of every target. E.g. during evaluation of generic abstract resolver class we know nothing about the class that is extending it. So getting full data need some hook that would be fired when all metadata from decorators is captured and before buildSchema call - I have no idea how to achieve that 😕

@goldcaddy77
Copy link

I'd also like to tap into the metadata. The thing I'm trying to solve is to dynamically generate the following from my schema:

  • <resource_name>CreateInput
  • <resource_name>WhereInput
  • <resource_name>UpdateInput
  • <resource_name>WhereUniqueInput
  • ...

...so that my API has extremely strong conventions. Your workaround above doesn't solve this issue. It would make more sense to tap into the schema generation step to read from the metadata and dynamically add to the graphql schema.

@MichalLytek
Copy link
Owner

The problem with

tap into the schema generation step to read from the metadata and dynamically add to the graphql schema

is that when doing the manipulation programatically, you loose all the typings, so you end up with redeclaring the interfaces for CreateInput or WhereUniqueInput which is against the goal of TypeGraphQL.

The better solution for derived types are types operator - Pick, Omit, Partial, Required, just like in TS. So in this way you can declare witch part of other type you want and add it to your class, so you can metadata for the schema and types for TS all in one 😉

Also the base types idea might be also helpful in managing this common relation between object type and input (subset of fields):

class BaseUserData {
  @Field()
  name: string;

  @Field(type => Int)
  age: number;
}

@ObjectType()
class User extends BaseUserData {
  @Field(type => ID)
  id: string;

  @Field(type => [Post])
  posts: Post[];
}

@InputType()
class UserCreateInput extends BaseUserData {
  @Field()
  someInputField: string;
}

@InputType()
class UserUpdateInput extends Partial(UserCreateInput) {}

@Input()
class UserWhereInput extends Partial(UserCreateInput) {
  @Field(type => [String])
  ids: string[]
}

This example is not really a good real example but just to show the API proposal 😉

@goldcaddy77
Copy link

@19majkel94 I appreciate the thoughtful write-up and good point about the types - I want those to be available, too. Currently, I'm achieving everything through type-graphql decorators like so:

@InputType()
export class UserWhereInput extends BaseWhereInput {
  @Field({ nullable: true })
  firstName_eq?: string;
  @Field({ nullable: true })
  firstName_contains?: string;
  @Field({ nullable: true })
  firstName_startsWith?: string;
  @Field({ nullable: true })
  firstName_endsWith?: string;

  @Field({ nullable: true })
  lastName_eq?: string;
  @Field({ nullable: true })
  lastName_contains?: string;
  @Field({ nullable: true })
  lastName_startsWith?: string;
  @Field({ nullable: true })
  lastName_endsWith?: string;

  @Field({ nullable: true })
  email_eq?: string;
  @Field(type => [String], { nullable: true })
  email_in?: string[];
  @Field({ nullable: true })
  email_contains?: string;
  @Field({ nullable: true })
  email_startsWith?: string;
  @Field({ nullable: true })
  email_endsWith?: string;

  ...more sections - one per filterable field
}

You can see that there is a lot of redundancy here and plenty of opportunity for developers to accidentally introduce inconsistencies in the API. This is the reason I'd like to auto-generate this piece. If you know of a magic way to do so within the framework that would be great. Otherwise I might need to just read the model metadata and auto-generate the schema and typings for this piece.

What I'm essentially trying to build is a Prisma-like framework that gives us a lot more control over our data models, but auto-generates all of the inputs and enums for consistency and velocity.

@MichalLytek
Copy link
Owner

MichalLytek commented Dec 25, 2018

The main problem is that TypeScript types system doesn't allow for creating dynamic property names:

A computed property name in an interface must refer to an expression whose type is a literal type or a 'unique symbol' type.

So as long as we can do this:

const prefix = "User";

interface UserFindOptions {
  [prefix]: boolean;
}

we can't do this:

const prefix = "User";
const createUser = `create${prefix}`;

interface UserFindOptions {
  [createUser]: string;
}

So we're not able to create a generic or a type operator that would generate this kind of interface.

But in your example, you can just use nested inputs! 😉

@InputType()
class UserWhereInput  {
  @Field({ nullable: true })
  firstName?: StringSearchOptions;

  @Field({ nullable: true })
  lastName?: StringSearchOptions;

  @Field({ nullable: true })
  email?: StringSearchOptions;
}

@InputType()
class StringSearchOptions {
  @Field({ nullable: true })
  eq?: string;

  @Field({ nullable: true })
  contains?: string;

  @Field({ nullable: true })
  startsWith?: string;

  @Field({ nullable: true })
  endsWith?: string;
}

So to make that typesafe, we can create a base type to implement:

type WhereOptions<T extends object> = {
  [P in keyof T]: StringSearchOptions | NumberSearchOptions | null;
};

@InputType()
class UserWhereInput implements WhereOptions<User> {
  @Field({ nullable: true })
  firstName: StringSearchOptions | null;
  
  @Field({ nullable: true })
  lastName: StringSearchOptions | null;
}

And this one would be easier to create dynamically using type transform operators or derived type util, as the property keys are known at the compile time.

For now you would have to setup a template and generate this code files from the model definition to avoid additional manual work.

@goldcaddy77
Copy link

@19majkel94 - thanks again for all of your detailed and thoughtful write-ups. It gives me a lot of confidence using your library. Your idea gets things one step closer to being fail proof, but still presents the opportunity for developer error. So I actually went ahead and did the auto-generation approach as you said and it works like a charm. It requires the schema generation step to be separate from the app running, but I'm happy to work around that to enforce consistency in my APIs.

Thanks again for all of the thought you've put into my problem!

@goldcaddy77
Copy link

FYI, for anybody following the 2nd half of this thread looking for something like I've outlined above, I implemented it in a new library Warthog that composes TypeGraphQL and TypeOrm, then layers these conventions on top.

@glen-84

This comment has been minimized.

@MichalLytek
Copy link
Owner

My current use case is generating a TypeORM query builder, where I need access to the entity property name when given a schema name.

@glen-84
So you need #123 rather than access to unbuilt and unprocessed metadata.

@kodeine
Copy link

kodeine commented May 9, 2020

The main problem is that TypeScript types system doesn't allow for creating dynamic property names:

A computed property name in an interface must refer to an expression whose type is a literal type or a 'unique symbol' type.

So as long as we can do this:

const prefix = "User";

interface UserFindOptions {
  [prefix]: boolean;
}

we can't do this:

const prefix = "User";
const createUser = `create${prefix}`;

interface UserFindOptions {
  [createUser]: string;
}

So we're not able to create a generic or a type operator that would generate this kind of interface.

But in your example, you can just use nested inputs! 😉

@InputType()
class UserWhereInput  {
  @Field({ nullable: true })
  firstName?: StringSearchOptions;

  @Field({ nullable: true })
  lastName?: StringSearchOptions;

  @Field({ nullable: true })
  email?: StringSearchOptions;
}

@InputType()
class StringSearchOptions {
  @Field({ nullable: true })
  eq?: string;

  @Field({ nullable: true })
  contains?: string;

  @Field({ nullable: true })
  startsWith?: string;

  @Field({ nullable: true })
  endsWith?: string;
}

So to make that typesafe, we can create a base type to implement:

type WhereOptions<T extends object> = {
  [P in keyof T]: StringSearchOptions | NumberSearchOptions | null;
};

@InputType()
class UserWhereInput implements WhereOptions<User> {
  @Field({ nullable: true })
  firstName: StringSearchOptions | null;
  
  @Field({ nullable: true })
  lastName: StringSearchOptions | null;
}

And this one would be easier to create dynamically using type transform operators or derived type util, as the property keys are known at the compile time.

For now you would have to setup a template and generate this code files from the model definition to avoid additional manual work.

@MichalLytek can you please give an example how we can use this where filter in find() method or typeorm?

    @Query(() => [classRef], {name: `all${classRef.name}s`})
    async findAll(): Promise<T[]> {
      return this.repo.find();
    }

@chrisbrantley
Copy link

@MichalLytek I'm dealing with having a ton of duplication between my ObjectTypes and Create and Update InputTypes. I think your NullableHOC strategy would help but I'm having a tough time wrapping me head around it.

You mentioned documenting that feature. Any chance I could get more detail on how that works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Discussion 💬 Brainstorm about the idea Enhancement 🆕 New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants