Redesign type/schema API to remove instanceof checks

[Pajn]

Per the discussion in #377

The current behavior makes it impossible to use graphql with npm links which is a huge problem if you have multiple interdependent packages as you'll have to rerun npm install for every single change and waste a lot of time.

[stubailo]

Looks like this is because of a limitation in Flow?

[taion]

BTW, this is probably not an unfamiliar limitation, but I believe you can work around this by using symlinks in the linked packages.

[Pajn]

By limitation in Flow, do you mean that it's something that first needs to be fixed on their side before the instance of checks can be removed? Should I file an issue?
I don't know Flow (we uses TS instead) but I would expect that there is a way to assert the type without affecting non-flow users.

Thanks for the tip about symlinking graphql, that certainly helps!

[josephsavona]

We've run into this a bunch during Relay development as well. I think the answer might be to fix npm though.

[leebyron]

Yeah - unfortunately we currently rely on instanceof both for how graphql works, but also because of how Flow works.

I don't think it's crazy to change this in the near future, but that would be a very real breaking change for all those dependent on the current types API.

[glasser]

We ran into an issue where we got:

Error: Can only create NonNull of a Nullable GraphQLType but got: ID. 
at invariant (node_modules/graphql/jsutils/invariant.js:19:11) 
at new GraphQLNonNull (node_modules/graphql/type/definition.js:651:29) 

ie, it was telling us that we weren't allowed to say ID!. This turned out to be multiple loaded versions of graphql and failing instanceof checks. Is that what this issue is about?

If so, maybe one partial solution would be to make graphql detect multiple loaded versions and print a warning? I believe React does that.

[jquense]

Hi ya'll, This is really annoying issue. Would everyone be open to leveraging Symbol.hasInstance to maybe address this? It should work for newer versions of node without breaking anything. e.g

class GraphQLObjectType {
  static [Symbol.hasInstance](foo) {
    return foo && foo[Symbol.for('GraphQLObjectType')];
  }
}

GraphQLObjectType.prototype[Symbol.for('GraphQLObjectType')] = true;

Should Allow the instanceof checks to continue to work without relying on the default JS behavior for them

[felixfbecker]

This has bitten me multiple times now:

• TypeError: type.getFields is not a function amount/gql2ts#85
• Equality check always returns false even if schema didn't change Urigo/graphql-cli#23
  the worst part about this was that the assertion here has no error message, it just fails with an empty string.

It's almost impossible to not end up with multiple graphql versions when using a lot of graphql tooling, because graphqljs is still 0.x, so it is not under semver and npm treats the difference between 0.15 and 0.16 as a breaking version, resulting in package duplication if one package depends on ^0.16 and one on ^0.15.

instanceof is really a bad practice in a language that uses ducktyping and node module resolution. You should never care about what class instance an object is, you should only care about whether it provides the properties you need.

[amannn]

If somebody is having trouble with this, I just learned that yarn supports forcing all dependencies to resolve to the same version. You can add this to your package.json:

"resolutions": {
  "graphql": "0.11.7"
}

If somebody runs into errors like Schema must be an instance of GraphQLSchema. Also ensure that there are not multiple versions of GraphQL installed in your node_modules directory. this can be a helpful workaround.

[leebyron]

Great suggestion @amannn - that's exactly what we do at Facebook for this module and a few others where having a single version is important.