graphql-modules icon indicating copy to clipboard operation
graphql-modules copied to clipboard

Can't use TS typedefs on an ESM setup

Open basicdays opened this issue 1 year ago • 7 comments

Describe the bug

I am not able to use the TypeScript type definition files when my Node 18 project is setup with ESM support enabled. TypeScript compilation will produce the following error:

Could not find a declaration file for module 'graphql-modules'. '/workspace/node_modules/graphql-modules/index.mjs' implicitly has an 'any' type.
There are types at '/workspace/node_modules/graphql-modules/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'graphql-modules' library may need to update its package.json or typings.

To Reproduce

Steps to reproduce the behavior:

package.json (abbreviated):

{
  "type": "module",
  "devDependencies": {
    "typescript": "~5.0"
  }
}

tsconfig.json (abbreviated):

{
  "compilerOptions": {
    "module": "node16",
    "moduleResolution": "node16"
  }
}

In any ts file the following will produce the TypeScript error:

import { createModule, gql } from "graphql-modules";

Expected behavior

An import should use the expected type definition files.

Environment:

  • OS: MacOS 12.6.6
  • @graphql-modules/...: 2.1.2
  • NodeJS: 18.16.0

Additional context

Was able to find some documentation on how to reference typedefs in the new exports feature: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-referencing

For what it's worth looking at the current graphql-modules package.json file, it currently has the following:

{
  "main": "index.js",
  "module": "index.mjs",
  "typings": "index.d.ts",
  "typescript": {
    "definition": "index.d.ts"
  },
  "exports": {
    ".": {
      "require": "./index.js",
      "import": "./index.mjs"
    },
    "./*": {
      "require": "./*.js",
      "import": "./*.mjs"
    },
    "./package.json": "./package.json"
  }
}

The graphql package at version 16.6.0 seems to be working correctly. It currently has the following:

{
  "main": "index",
  "module": "index.mjs",
  "typesVersions": {
    ">=4.1.0": {
      "*": [
        "*"
      ]
    },
    "*": {
      "*": [
        "NotSupportedTSVersion.d.ts"
      ]
    }
  }
}

basicdays avatar Jun 09 '23 18:06 basicdays

My current workaround is just to do the following when importing:

import graphqlModules = require("graphql-modules");

const { gql, createModule } = graphqlModules;

basicdays avatar Jun 09 '23 18:06 basicdays

Good news, I edited the graphql-modules/package.json file in my node_modules directly and got it working with the following:

{
  "main": "index.js",
  "module": "index.mjs",
  "typings": "index.d.ts",
  "typescript": {
    "definition": "index.d.ts"
  },
  "exports": {
    ".": {
      "require": {
        "types": "./index.d.ts",
        "default": "./index.js"
      },
      "import": {
        "types": "./index.d.ts",
        "default":"./index.mjs"
      }
    },
    "./*": {
      "require": {
        "types": "./*.d.ts",
        "default": "./*.js"
      },
      "import": {
        "types": "./*.d.ts",
        "default": "./*.mjs"
      }
    },
    "./package.json": "./package.json"
  }
}

basicdays avatar Jun 09 '23 18:06 basicdays

So I just revisted this again. Was going to do a PR to fix this, but I noticed that the package.json in the source includes the proper typings: https://github.com/Urigo/graphql-modules/blob/master/packages/graphql-modules/package.json#L21

However the package.json you get from the package published to npm is missing the exports["."].types and exports["./*"].types fields. I've used patch-package to at least rectify this for now, but is this a bug in the publishing process that's changing them? Does the package just need to get republished to get the correct exports?

Here's the patch-package diff for reference:

diff --git a/node_modules/graphql-modules/package.json b/node_modules/graphql-modules/package.json
index 00c64ab..c5b45b2 100644
--- a/node_modules/graphql-modules/package.json
+++ b/node_modules/graphql-modules/package.json
@@ -30,11 +30,13 @@
   "exports": {
     ".": {
       "require": "./index.js",
-      "import": "./index.mjs"
+      "import": "./index.mjs",
+      "types": "./index.d.ts"
     },
     "./*": {
       "require": "./*.js",
-      "import": "./*.mjs"
+      "import": "./*.mjs",
+      "types": "./*.d.ts"
     },
     "./package.json": "./package.json"
   }

basicdays avatar Jun 23 '23 19:06 basicdays

@patrickjm could you please take a look at this. It seems you fixed types in source code in this PR https://github.com/Urigo/graphql-modules/pull/2324 but the final package.json doesn't have that fix

wKich avatar Sep 07 '23 12:09 wKich

Any updates on this @patrickjm ? These are the exports from the published npmjs package, they seem to miss the types: image

itpropro avatar Nov 26 '23 23:11 itpropro

No updates here, would recommend making another PR for this one.

patrickjm avatar Nov 27 '23 02:11 patrickjm

No updates here, would recommend making another PR for this one.

The source code looks fine, this seems to be connected to the publishing pipeline.

itpropro avatar Dec 02 '23 01:12 itpropro