Closed cross311 closed 5 years ago
Been trying to work around this, and have found another workaround. javascript compiler is good enough that if the file is not in the import tree then the decorators will never get called.
Using the glob resolver syntax the only time a resolver will get loaded is if it is in that glob. Then you have to make sure that you do that use object types outside of the resolver flow. If an object type is used in a commonplace then it will get read in and be in the schema.
This workaround takes a lot of discipline but is possible.
There are a few things you need to know:
1) Some people want to reuse the types/args classes so they have it files in separate folders and even projects (#69). So as we have multiple node_modules
folders with multiple type-graphql
packages, we need to store the metadata attached to global
variable to share the metadata between packages inside single Node.js process.
2) Decorators has to be evaluated to put the metadata in store. So to be evaluated, they have to be in import chain from your app entry point. That's why you need to explicitly import resolvers classes or provide a glob string that will do that for you. Then the import chain goes to all imported args, input or object types and register them in metadata storage.
3) If the file with class is not in import chain, it won't be registered. It's usually unheard but if you use InterfaceType
, your ObjectType
implementation might be not imported in some case, so you might then need to explicitly import them in your base resolver because without it they won't show in your schema:
4) To actually include the types in schema, I need to provide them manually as graphql-js
not always include all of them:
https://github.com/19majkel94/type-graphql/blob/a704b8b992e0fe45aea3c1a35ee47f0b81faf9d6/src/schema/schema-generator.ts#L334-L341
5) I haven't considered multiple schema in one process as a common case. Because of the reasons described above, you may experience leaks in types emitted in schema. I would have to figure out how to provide in the schema-generator
only the missing types, not the spread on all existing types. Maybe also I should filter queries and other handlers, not emit all from the MetadataStorage :wink:
Thanks for the amazing response! As you mentioned in number 6, one way would be filtering at the end, which I am sure would not be an easy task.
I assumed we were using the library in a weird / edge case and was super excited that we could actually get it to do what we wanted! If we keep running into schema leaks I might take a harder look at the schema generator to see if I could implement what you recomend. But right now it is not blocking us, so I probably wont get to it soon.
If anyone wants to use library in similar way to us before a fix comes out a few things for workaround:
Again thanks for the amazing library.
I will try to take this into account during metadata -> schema converting process redesign.
Shouldn't be too hard to implement it, as the metadata storage will handle only metadata from decorators and the metadata builder will build full types info based on loaded resolvers only. So schema generator will emit only the needed types, so separate types
property of buildSchema
should be provided to support orphaned types:
Because of this, we can not have multiple schemes in one application?
When will this be fixed?
@Fyzu You have a workaround described in https://github.com/19majkel94/type-graphql/issues/110#issuecomment-404023653.
When will this be fixed?
There's no ETA, however you can also contribute and create a PR for that 😉 Stop demanding, start supporting - it's an OSS, not a paid software.
Resolver's classes are not taken into account for now but will be in the future. They exists to force users to import the created classes to evaluate the decorators and register in schema.
Glob strings are resolved and files are require
d.
Published in type-graphql@0.18.0-beta.1
🎉
Please guys let me know if it works for you - npm install type-graphql@beta
😉
Does not leak anymore, but if I try to generate two schemas , both schemas will be the former generated schema 😞
Like in this repro: https://github.com/MichalLytek/type-graphql/issues/234
I tried both buildSchema/buildSchemaSync
Cannot reproduce 😕
import "reflect-metadata";
import { ApolloServer } from "apollo-server";
import * as path from "path";
import { buildSchema, Resolver, Query } from "../../src";
@Resolver()
class FirstResolver {
@Query()
firstQuery(): string {
return "First Query";
}
}
@Resolver()
class SecondResolver {
@Query()
secondQuery(): string {
return "Second Query";
}
}
async function bootstrap() {
const firstSchema = await buildSchema({
resolvers: [FirstResolver],
emitSchemaFile: path.resolve(__dirname, "first-schema.gql"),
});
const secondSchema = await buildSchema({
resolvers: [SecondResolver],
emitSchemaFile: path.resolve(__dirname, "second-schema.gql"),
});
const firstServer = new ApolloServer({ schema: firstSchema });
const secondServer = new ApolloServer({ schema: secondSchema });
const firstServerInfo = await firstServer.listen(4001);
console.log(`First server is running at ${firstServerInfo.url}`);
const secondServerInfo = await secondServer.listen(4002);
console.log(`Second server is running at ${secondServerInfo.url}`);
}
bootstrap();
This code works without any issue, as designed, without leaks.
Ok, I can confirm now, sorry about this.
Looks like there is some kind of a issue with apollo-server-express. I was trying to attach two graphql servers on the same express app under two different paths, and that was the cause. When I switched to apollo-server and start on two ports, it was fine.
Thank you very much @MichalLytek !
We are also in the same situation and I can confirm the beta version implements the "cleanup" of the schema. But now it appears that there is a case not treated correctly:
Can you please confirm this is true and if we might have a fix?
Thank you
we have a field resolver defined directly into the decorator
@irimiab What do you mean? Can you share a code snippets for that part?
In this case:
@InterfaceType({
resolveType: (media) => {
switch (media.type) {
case MediaTypeEnum.ImageMedia: return ImageMedia;
case MediaTypeEnum.VideoMedia: return VideoMedia;
default: throw new GraphqlError(`Unsupported media type: ${media.type}`, GraphqlErrorCode.InvalidArguments);
}
},
})
export class Media {
@Field(type => ID, { nullable: true, description: "The id of the media" })
public id: string;
@Field(type => MediaTypeEnum, { description: "The type of the media" })
public type: MediaTypeEnum;
}
@ObjectType({ implements: Media, description: "The image media type" })
export class ImageMedia implements Partial<Media> {
public id?: string;
public type?: MediaTypeEnum;
@Field(type => Int, { nullable: true, description: "The width of a image" })
public width?: number;
@Field(type => Int, { nullable: true, description: "The height of a image" })
public height?: number;
}
@ObjectType({ implements: Media, description: "The video media type" })
export class VideoMedia implements Partial<Media> {
public id?: string;
public type?: MediaTypeEnum;
@Field(type => Int, { nullable: true, description: "The duration of the video in seconds" })
public duration?: number;
}
@Resolver(of => Media)
export class MediaResolver {
@Query(returns => Media, { nullable: true })
public async media(@Arg("id", type => ID, { description: "The id of the 'Media' to get" }) id: string, @Root() root: any, @Ctx() ctx: any, @Info() info: any) {
return getMedia(id, root, ctx, info);
}
}
const mediaSchema = await buildSchema({
resolvers: [MediaResolver],
});
The generated schema does not contain the types "VideoMedia" or "ImageMedia".
Yes, it has to works that way - when it's not referenced directly as an output type in some field or query, it has to be registered manually using orphanedTypes
.
I just wonder if there are other cases than object types implementing an interface? Maybe we can just expose an implementedBy?: ClassType[]
decorator option for interface as a way to register implementing types... or maybe better approach would be to scan the objectTypesMetadata for implementing an interface we want to register?
I will thing about that in #183, for now use orphanedTypes
as a workaround 😉
I just tested and indeed this solves our issue.
The idea of an implementedBy
decorator is good, more elegant, but for our use case - when we generate multiple schemas based on the same models/classes - the orphanedTypes
is more useful (because this is called when the schema is actually built, not during "declaration").
But we still have an issue with the interfaces (which I thought is caused by the missing types in the schema): in the example above, when the field resolver for the interface runs, I get an error like this:
Runtime Object type "ImageMedia" is not a possible type for "Media"
When I return strings from the resolveType
function, I get this error:
Abstract type Media must resolve to an Object type at runtime for field MediaContainer.media with value <a value here>, received "undefined". Either the Locator type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.
This seems to happen only on nested types, somehow, but I can't understand exactly why.
This same code works well on version 0.17.5.
Any idea about this?
I managed to make it work by including all types based on interfaces in the orphanedTypes
list, event if they were otherwise referenced in resolvers and included in the schema.
I'm not sure I can provide an example for this, but the basic use case is that we are having objects with fields having abstract types. The type resolver doesn't "find" these implementations, even if they are referenced in other parts of the code, unless we put the types implementing the interface in the "orphanedTypes" list.
Anyway, the beta version is good enough for us now. Thanks
even if they were otherwise referenced in resolvers and included in the schema
we are having objects with fields having abstract types
Could you create a really simple reproduction example? I would need to sneak into metadata storage and schema generator to see why it doesn't get emitted 😕
We have a similar use case and this comment is just to leave a breadcrumb about something that helped us. https://github.com/MichalLytek/type-graphql/issues/110#issuecomment-404023653 rightly says:
Do not use index.ts files for models folder or resolver folder, brings everything in even if not actually used especially since most server side apps do not do tree shaking.
We do use barrel imports for our models, and so we were getting all types in both schemas. This wasn't a huge deal but was a bit untidy and annoying. We found that @graphql-tools/utils
has a pruneSchema
function: https://the-guild.dev/graphql/tools/docs/api/modules/utils_src#pruneschema This removes any types that are unused, based purely on analysis of the schema itself.
So we now do something like this:
import { pruneSchema } from "@graphql-tools/utils";
import { buildSchema, emitSchemaDefinitionFile } from "type-graphql";
// `options` does not have `emitSchemaFile` set.
const schema = await buildSchema(options);
const schemaFilePath = "some/path/schema.gql";
await emitSchemaDefinitionFile(schemaFilePath, pruneSchema(schema));
Is your feature request related to a problem? Please describe. We are placing resolvers in different folders for public or internal. We are deploying the same graphQL codebase twice, one marked as public and one internal. The public one does not have a number of revolvers, due to them only being internal. We are doing this so we do not expose our internal models publically.
While only the resolvers that we require are showing up in the schema (👍 ), all of the ObjectTypes in the codebase are.
Describe the solution you'd like The requested solution is to only put types that are used in the loaded resolvers.
Describe alternatives you've considered We can split the codebase and move shared models into an npm package but would prefer not.