mhart/aws4fetch

Typescript/export/module/package.json B.S.

Closed this issue · 14 comments

Cosmic-Ryver commented

I ran into this problem today as well. The exports field in package.json just needs a "types" entry.

Diff that solves the problem:

diff --git a/node_modules/aws4fetch/package.json b/node_modules/aws4fetch/package.json
index 4ea1e45..95842c7 100644
--- a/node_modules/aws4fetch/package.json
+++ b/node_modules/aws4fetch/package.json
@@ -14,7 +14,8 @@
       "worker": "./dist/aws4fetch.esm.js",
       "browser": "./dist/aws4fetch.umd.js",
       "require": "./dist/aws4fetch.cjs.js",
-      "default": "./dist/aws4fetch.umd.js"
+      "default": "./dist/aws4fetch.umd.js",
+      "types": "./dist/main.d.ts"
     }
   },
   "types": "dist/main.d.ts",

This was partially generated by patch-package.

  • 👍2
tomlarkworthy commented

I don't think so, I think every export needs one, it also says the types should come first, and I should not have deleted the top level one as that's a fallback

https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

mhart commented

I'm confused about why this is a problem suddenly – the types are declared – why does anything need to change here? Is it a TypeScript bug?

Cosmic-Ryver commented

I'm confused about why this is a problem suddenly – the types are declared – why does anything need to change here? Is it a TypeScript bug?

In our project, the issue occurred after converting to ES Modules. As I understand, it is not a TypeScript bug. Depending on configuration in tsconfig.json and package.json TypeScript will strictly respect the exports field of dependencies' package.json files.

import /** ... */ from "dep/path" or import "dep/path" when "dep/path" is not explicitly included in dep's exports will be reported as an error. The absence of "./path" from exports is interpreted to mean the path is non-existent, private, or otherwise not a valid entry point. Similarly, import /** ... */ from "dep/path" when "./path" is included in exports, but does not have a types entry, will produce the error we encountered. In this case, any path.d.ts file is considered private because it is not explicitly exported.

The solution for compatibility with projects like our own is to either add the explicit types entry, or go with @tomlarkworthy's solution if fine grained control over what users are allowed to import is not desired.

Reproduction.

tomlarkworthy commented

I think why now is Typescript 5 changed how it resolved. I think the motivation is that packages can distribute wildly different APIs per export, and thus need different types per export. Not sure why the top level type is not default though. I think @GusBuonv 's patch better captures there are not different type exports for this package

revmischa commented

I ran into this issue after switching from "moduleResolution": "node" to "moduleResolution": "bundler"

mattiaz9 commented

Also "moduleResolution": "nodenext" creates this problem.
This comment explains why there is this issue:
microsoft/TypeScript#51996 (comment)

Ehesp commented

I ran into this problem today as well. The exports field in package.json just needs a "types" entry.

Diff that solves the problem:

diff --git a/node_modules/aws4fetch/package.json b/node_modules/aws4fetch/package.json
index 4ea1e45..95842c7 100644
--- a/node_modules/aws4fetch/package.json
+++ b/node_modules/aws4fetch/package.json
@@ -14,7 +14,8 @@
       "worker": "./dist/aws4fetch.esm.js",
       "browser": "./dist/aws4fetch.umd.js",
       "require": "./dist/aws4fetch.cjs.js",
-      "default": "./dist/aws4fetch.umd.js"
+      "default": "./dist/aws4fetch.umd.js",
+      "types": "./dist/main.d.ts"
     }
   },
   "types": "dist/main.d.ts",

This was partially generated by patch-package.

You can't apply a patch via patch-package to a package.json file right?

EDIT: Oh, you can: npx patch-package aws4fetch --exclude 'nonthing' (ref)

alfaproject commented

I've just stumbled upon this issue as well after changing moduleResolution to bundler as we are in the process to convert from CommonJS to ESM ):

@mhart https://arethetypeswrong.github.io/?p=aws4fetch%401.0.17

  • 👍1
alfaproject commented

Sorry for the notifications and off-topic but was just wondering if this is abandoned and I should fork or look for some other fork/alternative. Last commit in master was ~2 years ago and this is an actual/obvious issue that already has a PR with a fix for quite some time.

  • 👍1
mhart commented

This should be fixed in v1.0.18-beta.2 – please give it a spin and let me know if it fixes your issues

https://arethetypeswrong.github.io/?p=aws4fetch%401.0.18-beta.2

Will publish as ``v1.0.18` if it doesn't break anyone's workflows

  • 👍1
alfaproject commented

@mhart it works perfectly, thanks!

mhart commented

Ok, feel like it's been enough time, lol. Have just published 1.0.18 (identical to 1.0.18-beta.2)

  • 👍1