-
Notifications
You must be signed in to change notification settings - Fork 5
Transforms can lead to "empty" types breaking schema #37
Comments
This is supposed to be addressed within healSchema by pruneSchema. This may be an edge case or regression? If you can make a minimal reproduction repository, I will get on this. |
This edge-case was mentioned in #16 but maybe never fixed. I was trying out various versions, including v6.4 but wasn't working. You can reproduce via: import { FilterToSchema, transformSchema } from "graphql-tools-fork";
import { buildSchemaSync, ObjectType, Query, Resolver } from "type-graphql";
@ObjectType()
class EmptyObject {}
@Resolver()
class SomeResolver {
@Query()
public someQuery(): EmptyObject {
return new EmptyObject();
}
}
const schema = buildSchemaSync({
resolvers: [SomeResolver]
});
it("removes types without any fields", async () => {
const transformedSchema = transformSchema(schema, [
new FilterToSchema(schema)
]);
const objectTypes = transformedSchema.getTypeMap();
expect(objectTypes).not.toHaveProperty("EmptyObject");
}); If you add workaround transform before |
If you are using FilterObjectFields or TransformObjectFields which use visitSchema under the hood, healSchema is called automatically. Otherwise, I think transforms should maybe be responsible for calling healSchema themselves. |
Closed in error, waiting for reproduction. on another note because the schema is cloned prior to every transformation, and cloning includes healing, you only hit this with the last transformation, assuming there has not been a regression. |
Ah. This is because you are not using FilterObjectFields or TransformObjectFields. You can call healSchema directly prior to transformation if you want. I think that is a better solution, because the schema is invalid prior to transformation anyway, not really the role of transformSchema to fix schemas out of spec per se, that is what healSchema is for. Why do you have an empty type like this? |
I'm using (exactly in the specified order):
I think implementation details aren't important, hiding is based on custom decorators and metadata. I kinda half-expected FilterToSchema to take care of empty types and healing, probably wrongly. |
I don't. Type ends up being empty in the process of other transformations. |
FilterTypes uses visitSchema as well which should heal automatically. If you could make a minimal reproduction, I would be happy to take a look. It helps me tremendously, because then I can simply add to test suite. If you want to send a PR with failing test, that would also be great. As an aside, FilterToSchema is a default transform that you usually should not have to call. |
FilterToSchema is a request transform and not a schema transform. I am not sure whether it removes from the request empty objects at that point. I am going to work on making sure delegateToSchema does not error on entirely empty queries, something helpful for type merging |
Hmm, it's possible that FilterToSchema does literally nothing in my case because the transformed schema is also the target and the code has been wrong since the beginning in that place. The thing is, a type ends up being empty in the process of other transformations. I just did a quick check and calling So reproduction can become import { healSchema } from "graphql-tools-fork";
import { buildSchemaSync, ObjectType, Query, Resolver } from "type-graphql";
@ObjectType()
class EmptyObject {}
@Resolver()
class SomeResolver {
@Query()
public someQuery(): EmptyObject {
return new EmptyObject();
}
}
const schema = buildSchemaSync({
resolvers: [SomeResolver]
});
it("removes types without any fields", async () => {
const healedSchema = healSchema(schema);
const objectTypes = healedSchema.getTypeMap();
expect(objectTypes).not.toHaveProperty("EmptyObject");
}); |
Ahah. My guess/hope is interaction with type-graphql and healing code. Will take a look. |
@AndKiel I hope working for you? |
Yes, everything seems to be working. I can call |
Given:
When:
Then:
My current workaround is removing empty types through FilterTypes transform:
The question is, should this be the responsibility of
FilterToSchema
transform to clean up empty types as an additional step? It's already hiding queries and mutations that use types which were hidden, I think (or something like that).The text was updated successfully, but these errors were encountered: