slicknode / graphql-query-complexity

GraphQL query complexity analysis and validation for graphql-js
MIT License
710 stars 41 forks source link

v1.0.0 incompatible with Node.js at runtime #93

Open jaydenseric opened 3 weeks ago

jaydenseric commented 3 weeks ago

The recent v1.0.0 release has made this package incompatible with the Node.js runtime, because of this change from a valid fully specified import path to an invalid non fully specified import path missing the file extension:

https://github.com/slicknode/graphql-query-complexity/pull/91/files#diff-720574529a576948113b0f865801527a139a756ce25203d58f98f714bfd59b76R11

With Node.js v22.3.0, here is the runtime error:

node:internal/modules/run_main:115
    triggerUncaughtException(
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '[redacted]/node_modules/graphql/execution/values' imported from [redacted]/node_modules/graphql-query-complexity/dist/esm/QueryComplexity.js
Did you mean to import "graphql/execution/values.js"?
    at finalizeResolution (node:internal/modules/esm/resolve:260:11)
    at moduleResolve (node:internal/modules/esm/resolve:920:10)
    at defaultResolve (node:internal/modules/esm/resolve:1119:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:541:12)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:510:25)
    at ModuleLoader.getModuleJob (node:internal/modules/esm/loader:240:38)
    at ModuleJob._link (node:internal/modules/esm/module_job:126:49) {
  code: 'ERR_MODULE_NOT_FOUND',
  url: 'file://[redacted]/node_modules/graphql/execution/values'
}

We do not have a bundler in our Node.js GraphQL API project, and even if we did, we would be enforcing modern ESM module resolution rules which require fully specified import paths.

We will have to keep our dependency on graphql-query-complexity at ^0.12.0 until this issue is resolved.

Because the ecosystem can't be importing both ESM and CJS versions of graphql modules, so far it has settled on only importing the CJS because that's what came first and took root. The people best positioned to effect a transition to the ESM is the graphql maintainers, who could stop publishing CJS in a new major version and only publish pure ESM and force an ecosystem migration to align to ESM instead. That was actually planned for a major graphql release, but they chickened out on it in the end.

As a package author, you have to pick either importing ESM or CJS from graphql; you can't avoid making a decision.

jaydenseric commented 3 weeks ago

I see you're publishing both CJS and ESM modules in graphql-query-complexity:

https://unpkg.com/browse/graphql-query-complexity@1.0.0/dist/

Something you could consider, is having a build step when creating those artifacts that inserts .mjs file extensions in the graphql import paths when generating ESM modules, and .js when generating CJS. In the past I published a Babel plugin to help with this sort of thing:

https://github.com/jaydenseric/babel-plugin-transform-require-extensions

Nowadays in my own packages I militantly publish pure ESM and haven't used it for some time.

The catch with the above approach is that people (like me) like to consume your package as ESM, but expect it to pull in the CJS version of graphql. For us to continue using the CJS version of graphql (only to prevent duel package hazards with other ecosystem packages; our codebase is actually pure ESM) we would have to deep import the CJS version of graphql-query-complexity. Your package exports field would need to be updated to allow this, because currently it prevents any deep imports:

https://github.com/slicknode/graphql-query-complexity/blob/455978d4ed2934d7160a457965ab104998a3c515/package.json#L46-L51

For optimal JavaScript module design you should have package exports allowing deep imports of only the things people need anyway, and ideally remove the ability for consumers to import any other way. Force them to fall into the pit of success.

hprathap commented 4 days ago

+1 for this