Bug: FilterSchema does't remove type when all fields filtered (Unit tests is incorrect)

[ntziolis]

This test description implies that a type should be filtered out when all fields of the type are filtered out.
https://github.com/Urigo/graphql-mesh/blob/21643d748cac4ed95663f4f41f536aec4dcce02c/packages/transforms/filter-schema/test/transform.spec.ts#L68

However in the string the unit tests checks against the type still does exist:
https://github.com/Urigo/graphql-mesh/blob/21643d748cac4ed95663f4f41f536aec4dcce02c/packages/transforms/filter-schema/test/transform.spec.ts#L97

In our case we want to filter out ALL mutations. However because the Mutation type is "left-over" we get the following error: Error: Type Mutation must define one or more fields.

When leaving at least one field within the Mutation type everything works as expected.

What we expected would happen is that the entire Mutation type would be removed. If this is not the expected behavior, how could we achieve this?

Our setup:

• nestjs
• nestjs graphql module that gets fed from graphql mesh
  

[ardatan]

@yaacovCR Should we remove the type when it has no fields on it? Or do you have any suggestion for this error?

[ntziolis]

If you decide not to remove the type by default. I would suggest either:

• a flag that enables removing types without fields.
• ability to remove root fields explicitly (Query/Mutation/Subscription)

[ardatan]

@ntziolis We just need to decide if we should remove empty types or remove them explicitly in another transform.

[yaacovCR]

You can use pruneSchema after filterSchema.

[ardatan]

@ntziolis Did you try to filter the type itself instead of emptying it?

filterSchema:
  - !Mutation

[ardatan]

@yaacovCR Thanks! Now it uses pruneSchema to clean the empty types as @yaacovCR suggested. Also @ntziolis is right, I fixed the tests.
56f4001
The patch release will be available soon!

[ardatan]

Available in @graphql-mesh/transform-filter-schema@0.8.21!

[ntziolis]

Wow, you guys are fast. I can confirm that its now working as excepted. Thank you.

@ardatan Not sure if it still matters, but yes I think we did try !Mutation as well with the same result.

[ntziolis]

Making sure this is not an unintended side effect:

• When removing all fields from an input via MyInput.!*
• This removes the args that use this input from all fields in the entire graph

The mesh starts correctly and I think this would also be the desired behavior.

However when:

• When removing the input via !MyInput
• All fields that use this input in their args will be removed, even if they have other args

This seems like it could be source of surprises for people. Out of curiosity:
What is the internal difference between removing all fields of a type and removing the type?

Somewhat related to:
#1481

[ardatan]

@ntziolis

This removes the args that use this input from all fields in the entire graph

The idea is to remove empty types so the fields and arguments that use this type when it is removed.

All fields that use this input in their args will be removed, even if they have other args

The second one looks like a different bug. Could you share a reproduction for the second one so we can decide if the issue is on Mesh side or graphql-tools side?

What is the internal difference between removing all fields of a type and removing the type?

https://github.com/Urigo/graphql-mesh/blob/master/packages/transforms/filter-schema/src/index.ts#L39
The first one uses FilterXFields transforms from graphql-tools while the other one uses FilterTypes.