Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package types is not being resolved in a TS setup when moduleResolution is bundler #2480

Open
raulfdm opened this issue Dec 10, 2023 · 7 comments · May be fixed by #2481
Open

Package types is not being resolved in a TS setup when moduleResolution is bundler #2480

raulfdm opened this issue Dec 10, 2023 · 7 comments · May be fixed by #2481

Comments

@raulfdm
Copy link

raulfdm commented Dec 10, 2023

Describe the bug

I've noticed that when using graphql-modules in a TypeScript project that defines moduleResolution: "bundler":

{
  "compilerOptions": {
    // ... other config
    "moduleResolution": "Bundler",
    "module": "ESNext",
    "noEmit": true,
  }
}

TypeScript cannot resolve the type definitions:

CleanShot 2023-12-10 at 09 13 41@2x

That's because in the generated dist/package.json, the types field is not present in the exports condition:

image

I won't write all the details here because I saw you're using a tool called bob to generate the final dist, and the problem is there.

I just opened this issue to keep it as a reminder so that when this problem is fixed on the bob's side, you can upgrade it and generate a release.

To Reproduce

  1. Clone this repository: https://github.com/devraul/graphql-modules-type-problem
  2. Follow the README instructions

Expected behavior

Working without a problem in TS configs where it defines the moduleResolution equals bundler

Environment:

  • OS: macOS
  • graphql-modules: 2.3.0
  • NodeJS: 20.10.0

Additional context

@raulfdm
Copy link
Author

raulfdm commented Dec 10, 2023

A video demo:

CleanShot.2023-12-10.at.09.40.09.mp4

@raulfdm raulfdm linked a pull request Dec 11, 2023 that will close this issue
12 tasks
@zachariahtimothy
Copy link

Confirmed adding the types to the export sections fixes this. Wondering if we can do a shorter PR in the interim to just add these?

@raulfdm
Copy link
Author

raulfdm commented Dec 13, 2023

Confirmed adding the types to the export sections fixes this. Wondering if we can do a shorter PR in the interim to just add these?

The package.json of graphql-modules already declares the exports.type:

The problem is that the version of the bundler lib doesn't include this feature. Meaning that the only solution would be upgrading it.

@shishkin
Copy link

I was able to workaround this issue with a quick and dirty solution using patch-package and

diff --git a/node_modules/graphql-modules/package.json b/node_modules/graphql-modules/package.json
index 3feda76..d37c95f 100644
--- a/node_modules/graphql-modules/package.json
+++ b/node_modules/graphql-modules/package.json
@@ -30,7 +30,8 @@
   "exports": {
     ".": {
       "require": "./index.js",
-      "import": "./index.mjs"
+      "import": "./index.mjs",
+      "types": "./index.d.ts"
     },
     "./*": {
       "require": "./*.js",

@Wroud
Copy link

Wroud commented Apr 20, 2024

same problem here

@loicnestler
Copy link

Any updates on this? This is killing my ci/cd pipeline :/

@ericclemmons
Copy link

I was able to workaround this issue with a quick and dirty solution using patch-package and

diff --git a/node_modules/graphql-modules/package.json b/node_modules/graphql-modules/package.json
index 3feda76..d37c95f 100644
--- a/node_modules/graphql-modules/package.json
+++ b/node_modules/graphql-modules/package.json
@@ -30,7 +30,8 @@
   "exports": {
     ".": {
       "require": "./index.js",
-      "import": "./index.mjs"
+      "import": "./index.mjs",
+      "types": "./index.d.ts"
     },
     "./*": {
       "require": "./*.js",

This worked perfectly! pnpm patch graphql-modules

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants