Remove some instanceof checks

[mjmahone]

If you end up in the unfortunate situation of having multiple graphql-js node_modules installed, instanceof completely fails. While it's almost always preferable to reduce your node_modules installations so you don't have multiple instances of the same exact code, it may not always be possible. This fixes some of the cases for where it's not possible, specifically in util functions: those are the most likely to be shared across installation instances.

To fix, we're adding kind field to every Schema type class, so we can compare via type.kind === 'GraphQLObjectType instead of type instanceof GraphQLObjectType.

Note, there are still many many uses of instanceof in graphql-js, so it's still not safe to have two instances installed.

[JenniferWang]

@facebook-github-bot import

[facebook-github-bot]

Sorry, I can't do that @JenniferWang - this feature is only supported for non-employees; you can land via the internal pull request dashboard.

[wincent]

Note, there are still many many uses of instanceof in graphql-js, so it's still not safe to have two instances installed.

I'd feel better about this if we did it comprehensively, although I know you're probably wanting to do the minimal amount of work to unblock yourself. Fun fact. If this makes it into a GraphQL update and you deploy the update in an environment with the old version still present somewhere, any code path that hits the old version will still hit the instanceof checks and fail, so you'll have to dedupe anyway.

What do you think of having a module with a bunch of predicate functions (isGraphQLObjectType etc)? Would be nice to have those string comparisons centralized in one place, as well as somewhere we could make the checks more stringent if we wanted to (eg. verifying more than just the kind), and potentially use some Flow %checks annotations to make the typing nicer elsewhere in the system.

[wincent]

@JenniferWang: Don't listen to the bot, we don't import in this repo anyway. The only way to get code deployed internally is to merge it here, cut a release, then install the release.

Bear in mind, however, that this is the reference implementation relied on by many third-party (and FB) software products, so there's no way we should be merging without the CI being green, and ideally without allowing time for adequate review.

[schickling]

@wincent would be really fantastic if this could be merged as this is one of the biggest time sinks when developing GraphQL tooling by linking NPM modules.

[leebyron]

I'm going to close this since tests are failing and it's not really a comprehensive solution.

In my opinion this is more of a bandaid than a real solution since the real problem is that this package evolves in capabilities over time and the types and classes exported by one version may not behave correctly with functions from another version. In addition there are deep assumptions made in the package about singletons like build in scalar types or introspection types which we check via identity. Even if we were to change instanceof to brand-checks like this, we're still stuck with multiple versions resulting in weird or broken behavior.

I also much prefer @wincent's suggestion of predicate functions. That provides an improved API and could allow for a centralized place to produce more descriptive error messages about duplicate graphql-js packages.

[schickling]

I also much prefer @wincent's suggestion of predicate functions. That provides an improved API and could allow for a centralized place to produce more descriptive error messages about duplicate graphql-js packages.

I completely agree. @leebyron can you provide some background on what's the next step here? Is that something Facebook is planning to work on or are you suggesting that someone of the community should tackle this?

I've come across this specific problem a dozen of times and I'm sure other library maintainers have experienced the same issues when multiple graphql-js versions are involved because of some dependency (this especially becomes a problem when linking modules).