graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.04k stars 2.02k forks source link

Error: Ensure that there are not multiple versions of GraphQL installed in your node_modules directory #594

Closed richburdon closed 7 years ago

richburdon commented 7 years ago

[Related to https://github.com/graphql/graphiql/issues/58]

I have two node modules A => B, where B creates a GraphQLSchema object that is imported by A, which is then used to instantiate graphqlExpress.

I get this error: Ensure that there are not multiple versions of GraphQL installed in your node_modules directory

And see that I have 2 different instances of the GraphQLSchema class type at runtime -- so validate.js invariant(schema instanceof GraphQLSchema) fails because the imported schema is an instance of A's GraphQLSchema not B's GraphQLSchema.

However, all npm dependencies are of the same version (in both A and B modules).

> npm --version
3.10.7

> babel-node --version
6.16.0

> find node_modules -name graphql
node_modules/graphql
node_modules/graphql-subscriptions/node_modules/graphql

> grep version node_modules/graphql/package.json 
0.7.2

> grep version node_modules/graphql-subscriptions/node_modules/graphql/package.json
0.7.2

I assume this is a common pattern, but I can't find an example.

BTW, I'm using npm-link to create the A => B dependency.

richburdon commented 7 years ago

Using peerDependencies in A/package.json I managed to get rid of node_modules/graphql-subscriptions/node_modules/graphql:

  "peerDependencies": {
    "graphql": "^0.7.0",
    "graphql-subscriptions": "^0.2.1"
  }

But the problem remains since A's GraphQLSchema class is different from B's.

stubailo commented 7 years ago

This seems like a problem with npm or possibly the graphql-subscriptions package, not with graphql-js.

richburdon commented 7 years ago

Probably right, but this seemed like a common pattern?

stubailo commented 7 years ago

Sure, I guess I'm just saying there isn't much graphql-js can do about this. It's a good thing that it checks that you only have one instance, otherwise there would be much worse problems.

richburdon commented 7 years ago

I think this is the solution (if you're using webpack):

  resolve: {
    alias: {
      graphql:  path.resolve('./node_modules/graphql'),
      react:    path.resolve('./node_modules/react')                // Same issue.
    }
  }

http://stackoverflow.com/questions/31169760/how-to-avoid-react-loading-twice-with-webpack-when-developing

casoetan commented 7 years ago

Got this issue as well.

Using node 7.9

$ find node_modules -name graphql

node_modules/@types/graphql
node_modules/graphql

should i just delete the @types/graphql directory?

I don't think this should affect it but...

wincent commented 7 years ago

@casoetan: deleting it sounds like a reasonable bet.

jay-jlm commented 7 years ago

I get the same results of casoetan. deleting @types/graphql doesnt seem to have an effect

casoetan commented 7 years ago

The issue seems to be the way i called makeExecutableSchema. Solved this by using 'graphql-modules'.

richburdon commented 7 years ago

@stubailo I resolved this for webpack some time ago, but the problem still exists for monorepos that have different sub packages referencing modules that depend on graphql.

I tried lerna --hoist to resolve, but this is flaky.

(BTW, I removed the dependency on graphql-subscriptions (red herring).

I've also tried moving graphql deps into peerDependencies for my own sub packages that my failing package depends on.

In the module where the error occurs (as above):

find -L node_modules -name graphql
node_modules/@types/graphql
node_modules/test-api/node_modules/@types/graphql
node_modules/test-api/node_modules/test-core/node_modules/@types/graphql
node_modules/test-api/node_modules/test-core/node_modules/graphql
node_modules/test-client/node_modules/@types/graphql
node_modules/test-client/node_modules/test-api/node_modules/@types/graphql
node_modules/test-client/node_modules/test-api/node_modules/test-core/node_modules/@types/graphql
node_modules/test-client/node_modules/test-api/node_modules/test-core/node_modules/graphql
node_modules/test-client/node_modules/test-core/node_modules/@types/graphql
node_modules/test-client/node_modules/test-core/node_modules/graphql
node_modules/test-client/node_modules/graphql
node_modules/test-core/node_modules/@types/graphql
node_modules/test-core/node_modules/graphql
node_modules/graphql

where test-* are linked modules (all of which define graphql as peerDependencies.

I assume @types/graphql are normal TypeScript deps?

leebyron commented 7 years ago

Closing this aging issue.

graphql-js is not designed to support using functionality between different installations of itself. The instanceof is only one example where this failure occurs - but there are others as well, especially if there would ever be inadvertent differences in released versions.

If you use packages with sub-packages, ensure they support ranges of versions for graphql so that npm or yarn can find a single version to install at the top level which works for all dependents.

leebyron commented 7 years ago

A note for those still encountering this issue, yarn is pretty good at doing this version resolution, including support for a flag which enforces guarantees for a single version of every dependency if you want that. I believe npm5 also has some support for this kind of guarantee

webmobiles commented 7 years ago

I'm using yarn and have the same error

michaellee8 commented 7 years ago

I have also faced this error, for example, my express-graphql uses graphql@11.x.x but I had installed graphql@8.x.x due to graphql-sequelize-crud uses a old version of graphql. I checked this out by this command

$ find node_modules -name graphql
node_modules/express-graphql/node_modules/graphql
node_modules/graphql

So I just run rm -rf node_modules/express-graphql/node_modules/graphql here and everything solved. Note that basically you need to only have 1 line result for find node_modules -name graphql

jdachtera commented 6 years ago

I'm facing this problem today using yarn. I believe the source of this is that graphql-js is still using alpha versions (0.x...) and therefore the normal semver ranges don't apply. So #1005 needs to be fixed.

nishant-dani commented 3 years ago

I have only a single instance of graphql in node_modules.

I have a shared GraphQLObject instance that is created in a library and I have resolved that main: console.log(case 1): graphql path is ${require.resolve('graphql')}) and library: console.log(case 2): graphql path is ${require.resolve('graphql')}) both resolve to the same path.

Yet I get the instanceof error flagged - that somehow it detects that the GraphQLObject object are belonging to different graphl instances.

benjie commented 3 years ago

@nishant-dani Do you have a bundler or minifier corrupting the exports?

thekevinbrown commented 3 years ago

I have the following:

❯ find node_modules -name graphql
node_modules/graphql
❯ yarn list graphql
yarn list v1.22.5
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ graphql@15.4.0
✨  Done in 1.65s.

So there's definitely only one version, and yet:

Error: Cannot use GraphQLSchema "[object GraphQLSchema]" from another module or realm.

Ensure that there is only one instance of "graphql" in the node_modules
directory. If different versions of "graphql" are the dependencies of other
relied on modules, use "resolutions" to ensure only one version is installed.

https://yarnpkg.com/en/docs/selective-version-resolutions

Duplicate "graphql" modules cannot be used at the same time since different
versions may have different capabilities and behavior. The data from one
version used in the function from another could produce confusing and
spurious results.

I'm in a typescript monorepo, so there's no bundler or minifier, just the TS compiler. I have no idea what to do to isolate the issue.

jayeshmann commented 3 years ago

@thekevinbrown I am having the same issue, yours is fixed?

PatrykMilewski commented 3 years ago

Having the same issue as @thekevinbrown

benjie commented 3 years ago

One cause of this, other than having duplicate modules, is having graphql imported two different ways - e.g. having it imported as CommonJS in one location and having it imported as ESM in another location. This results in two independent sets of objects in memory and is one potential cause of this error. If you're using a bundler you should be able to have it show you what it's importing - if it's importing both of these files then this is your issue:

PatrykMilewski commented 3 years ago

In my case enforcing resolution to version 14 instead of 0.13 fixed the issue

thekevinbrown commented 2 years ago

@PatrykMilewski and @jayeshmann, in our case this was actually caused by Serverless Offline. The fix was, of all things, a configuration option allowCache: true like here: https://github.com/dherault/serverless-offline/issues/931#issuecomment-660740567

Absent this, Serverless Offline uses a type of threading to run the lambdas, and that's where the duplicates come from. There's also useChildProcesses but that's much slower than simply allowing it to have a require cache properly.

bombillazo commented 2 years ago

Is there a way to avoid this error even when having multiple installed versions? This is a nightmare to deal with with monorepos and toolings that all install their own version of GraphQL (Nx, Expo, graphql-tools, etc)...

yaacovCR commented 2 years ago

All libraries should install graphql-js as a peer dependency so that a separate version is not installed. If this is not the case for some library, it should be reported as a bug to the library developers.

Otherwise, resolutions can be used with some package managers to force dependencies to use a single version.

Explorations have begun to support multiple packages for cjs/esm support, but you normally should not run into problems. graphql-tools in particular, as far as I am aware, correctly installs this library as a peer.

benjie commented 2 years ago

All libraries should install graphql-js as a peer dependency so that a separate version is not installed. If this is not the case for some library, it should be reported as a bug to the library developers.

Are you certain this advice works with all package managers (npm, yarn, yarn pnp, pnpm, etc), with nested dependencies which each have a dependence on graphql, and in repositories that use workspaces? Personally I've found yarn pnp to be very picky about peerDependencies in the past, and even in non-pnp mode IIRC peerDependency warnings are output if packageA peerDepends on graphql and also depends on packageB which peerDepends on graphql and you only have graphql at the top level. I may be misremembering though...

yaacovCR commented 2 years ago

I'd certainly be open to learning more from a minimal reproduction of any issue with this approach, simply to further my own knowledge.

But I can't figure how the solution would be inside this library if particular package managers don't respect peer dependencies as peer-only?

alper commented 2 months ago

I get the error despite pnpm why graphql showing all versions to be the same.

There really is no fix for this?

alper commented 2 months ago

All libraries should install graphql-js as a peer dependency so that a separate version is not installed. If this is not the case for some library, it should be reported as a bug to the library developers.

This seems extremely unlikely that it would happen.

benjie commented 2 months ago

In yarn you can use the resolutions feature to ensure there's exactly one version of a module, you may find that pnpm has a similar feature?

thekevinbrown commented 2 months ago

Indeed it does: https://pnpm.io/package_json#pnpmoverrides

alper commented 2 months ago

Yes, we tried pnpm overriding it but it didn't work.

benjie commented 2 months ago

@alper Are you able to share a reproduction of it not working?

benjie commented 2 months ago

@JoviDeCroock What do you think about adding a .stack() or similar method to GraphQLSchema? This would allow us to call schema.stack?.() on each of the schemas which would help reveal where the duplication has come from - whether that be separate (duplicate) modules, or whether it be that one schema is from the CJS imports and one from the ESM imports.

benjie commented 2 months ago

Another option worth considering might be to hand over execution to the schema object itself, so graphql(), execute(), validate(), etc would all just pass down into schema.graphql(), schema.execute(), etc - that way if it "smells" like a schema, we can hand over to that schema's execution methods and not worry that it was called from a different version. Effectively the main exposed functions would just become a facade. Doesn't solve the issue, but at least hides it for most users hitting it. Still would mean they have two GraphQL versions in their bundles though, which is something they still ought to fix (e.g. with overrides/resolutions).

JoviDeCroock commented 2 months ago

Hmm, I agree that could be a great way to debug but for that to work wouldn't we have to construct that on every element of a GraphQLSchema? That would enable us to extend our instanceof check to also report the origin of the schema and of the element that is not part of the same realm.

I assume that we are using an ESM/CJS variant or two ESM variants with slightly different naming that duplicates the module re-reading the comments here. Assuming the resolutions are working fine.

benjie commented 2 months ago

There's separate CJS and ESM files that operate independent of each other, so if you import CJS in one location and ESM in another you get two full copies of GraphQL in memory. Not an ideal setup - in my opinion it would be better if the ESM was a facade over the CJS (although that would impact tree shaking no doubt) - but no-one has implemented a better setup yet.

But yes, I think if we did Object.defineProperty(GraphQLObjectType, '_stack', { value: stack }) on each of the objects we instanceof then it would really help improve this error message. Even better would be if we could isolate the first line of the stack and render an explicit comparison, but that is dependent on the JS engine in which it runs so probably not something we can reliably do.

I'm thinking that stack would be initialized something like:

let stack: string;
try {
  throw new Error(`GraphQL ${version} was initialized here`);
} catch (e) {
  stack = e.stack ?? String(e);
}
JoviDeCroock commented 2 months ago

That's a good shout, only in dev preferably as stack in NodeJS can be quite expensive but I wholeheartedly agree with the approach!

benjie commented 2 months ago

I figure if we're only doing it once on startup it shouldn't be too expensive?

thekevinbrown commented 2 months ago

Cold boot in Lambda would mean that'd happen quite a bit in our setup. My preference would be to only do it if a debug option is enabled somehow for someone who is having this issue and looking to find what the cause is.

benjie commented 2 months ago
node -e 'let stack; const start = process.hrtime.bigint(); try { throw new Error(`GraphQL ${"16.0.0"} was initialized here`); } catch (e) { stack = e.stack ?? String(e); }; const stop = process.hrtime.bigint(); const dur = Number(stop - start)/1e6; console.log(`${dur.toFixed(3)}ms`);'
0.085ms

Indeed; looks like it would slow startup time by as much as a twelth of a millisecond. Just capturing the error (so you can access the stack later if needed) is a few times faster:

node -e 'let stackE; const start = process.hrtime.bigint(); try { throw new Error(`GraphQL ${"16.0.0"} was initialized here`); } catch (e) { stackE = e; }; const stop = process.hrtime.bigint(); const dur = Number(stop - start)/1e6; console.log(`${dur.toFixed(3)}ms`);'
0.014ms