Graphql-tools: Can't build with Rollup due to Circular dependencies

[acoreyj]

Circular dependencies make building for browser with rollup impossible, you will get the below error in console.

Uncaught TypeError: Cannot read property 'checkForResolveTypeResolver' of undefined

Rollup outputs the following about the circular dependency

Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/addResolveFunctionsToSchema.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/addResolveFunctionsToSchema.js -> commonjs-proxy:/Users/Corey/Documents/workspace/mkr/examples/memory/node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/assertResolveFunctionsPresent.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/attachConnectorsToContext.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/buildSchemaFromTypeDefinitions.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/checkForResolveTypeResolver.js -> node_modules/graphql-tools/dist/generate/index.js
Circular dependency: node_modules/graphql-tools/dist/generate/index.js -> node_modules/graphql-tools/dist/generate/concatenateTypeDefs.js -> node_modules/graphql-tools/dist/generate/index.js

[stubailo]

I'd accept a PR to fix this, but making this work with Rollup specifically isn't a big priority since this is (1) primarily a server-side library and (2) circular dependencies are explicitly a part of JavaScript so hopefully Rollup can one day support them.

[stubailo]

Even better would be an automated test to check for Rollup compatibility, since this is very likely to regress in the future.

[dnalborczyk]

circular dependencies are explicitly a part of JavaScript so

I think this is a little too generalized. it depends on the module system (es6, cjs, amd, umd) one is using. I don't know rollup much, and not sure if the above is valid or not, but having circular dependencies sounds like code smell. it might also cause problems when graphql-tools is switching to es6 modules in the future.

[bennypowers]

The best way to fix this would be to distribute standard modules.

If a CJS build is needed, it could be automated with rollup.

I'd be happy to make a PR for that.

I can't at the moment use this library (for client-side code) in testing because it fails to build, due to the heavy transformations applied to the source

I'd accept a PR to fix this, but making this work with Rollup specifically isn't a big priority since this is (1) primarily a server-side library and (2) circular dependencies are explicitly a part of JavaScript so hopefully Rollup can one day support them.

This kind of comes off like you're saying "Rollup users and developers who write standard javascript don't matter". I'm sure that's not what you mean, but there are a lot of developers who would love to use your libraries if the barrier to entry was lowered a little (read: if they could just import it like normal).

The tide towards standard js is already turning with projects like pikapkg.com satisfying the desire for cruft-free JS. Please let the community help you ride that wave 〰️ .

[btakita]

It seems like the only way to get around this atm is to use commonjs instead of ESM (import) for anything related to graphql.

[btakita]

Is this project still active & accepting pull requests?

[langpavel]

This is not only about rollup. This is about every module which need call this library immediately, in initialization phase.

[yaacovCR]

Is this project still active & accepting pull requests?

Yes!

[yaacovCR]

Should be fixed by #1326, rolling into #1306