graphql-tools icon indicating copy to clipboard operation
graphql-tools copied to clipboard

filterSchema typeFilter unexpected behavior

Open ian opened this issue 3 years ago • 4 comments

Describe the bug When running filterSchema it should be keeping the type in its entirety, but it's removing all the fields from the type but keeping the type definition.

Running the typefilter (type) => type === "Post" on the the schema:

type Post {
  headline: String
  body: String
}

type User {
  name: String
  email: String
  posts: [Post]!
}

type Query {
  users: [User]!
  posts: [Post]
}

type Subscription {
  users: [User]!
}

Will result in the entire Post type to be removed:

type Post

type Query {
  posts: [Post]
}

type Subscription

Additionally, pruneSchema should never leave dangling types, in this case Subscription should be pruned since it does not include any properties.

To Reproduce See codesandbox https://codesandbox.io/s/nifty-gauss-2vbng?file=/src/index.js

Expected behavior If typeFilter is present and returns true for a type, the type should be maintained in it's entirety.

The expected output from the above statement is:

type Post {
  headline: String
  body: String
}

type Query {
  posts: [Post]
}

Environment: See codesandbox for package list.

ian avatar Sep 28 '21 14:09 ian

Interestingly enough, if you run the filtered schema through pruneSchema twice, it corrects the dangling subscription type.

ian avatar Sep 28 '21 18:09 ian

What happens if you define a fieldFilter that always returns true in addition to the typeFilter? It’s possible that it needs the fieldFilter present given the way all the filter gates stack; memory is foggy there. Your result with all fields removed from the type isn’t entirely surprising. Prune schema is definitely required to remove unreferenced types, though the fact that you have to run it twice sounds suspicious. Mind writing a failing test and dropping it into the ticket here?

gmac avatar Oct 03 '21 00:10 gmac

@ian finally got a chance to write a quick test for this and I see your problem now. You are aggressively filtering out all types not called Post, which includes the String scalar. Removing the String type from the schema correctly cascades down to all fields that implement it, thus leaving you with an empty Post type. This adjustment to your type filter should get you sorted out:

import { isSpecifiedScalarType } from 'graphql'

const filtered = filterSchema({
  schema,
  typeFilter: (typeName, type) => typeName === 'Post' || isSpecifiedScalarType(type),
});

@ardatan – I think we're good to close this one out. I don't think this merits any changes to the underlying libs, as filtering scalars under normal circumstances is a perfectly valid use case.

gmac avatar Oct 05 '21 13:10 gmac

Also regarding pruneSchema and the Subscription type, I suspect that's very specific to the "Subscription" type name being honored as a root operation by default, and therefore it has an implementation that may not be explicitly shown in the schema. Under the hood it would effectively be

schema {
  subscription: Subscription
}

You're probably emptying its members on the first pass, and then removing empty operations on the second pass. There's probably something out of order in mapSchema for handling empty operations too early in the process, and thus the results are not idempotent. Probably merits its own issue.

gmac avatar Oct 05 '21 13:10 gmac