panva / jose

JWA, JWS, JWE, JWT, JWK, JWKS for Node.js, Browser, Cloudflare Workers, Deno, Bun, and other Web-interoperable runtimes
MIT License
5.65k stars 316 forks source link

Module not found: Can't resolve 'jose/jwt/sign' #193

Closed qikevinjkw closed 3 years ago

qikevinjkw commented 3 years ago

Describe the bug

To Reproduce

  1. npm install jose
  2. Try to import like so: import { SignJWT } from "jose/jwt/sign";
  3. Typescript compiler errors: Module not found: Can't resolve 'jose/jwt/sign'

Code to reproduce the behaviour: https://stackblitz.com/edit/react-ts-kkyw3y

Expected behaviour Should import correctly with no errors

Environment:

panva commented 3 years ago

jose version: v3.11.6 affected runtime is: Node.js 14.15.0

works 🎉

❯ node -v
v14.15.0

❯ node -e 'import {SignJWT} from "jose/jwt/sign";console.log(SignJWT)' --input-type=module
[class SignJWT extends ProduceJWT]

I am unable to provide solutions for non-standard loaders or ecosystems that chose to build their own loaders are not keeping them up to date with node's own module resolution algorithm.

tellnes commented 3 years ago

I'm solving this problem by importing the relevant files directly. I put this into an intermediate file jose.ts and can then import like I expect to from that. Eg. like this:

import type * as T_SignJWT from 'jose/dist/types/jwt/sign'
export const { default: SignJWT } = require('jose/dist/node/cjs/jwt/sign') as typeof T_SignJWT

import type * as T_jwtVerify from 'jose/dist/types/jwt/verify'
export const { default: jwtVerify } = require('jose/dist/node/cjs/jwt/verify') as typeof T_jwtVerify

And then I can use it like this:

import { SignJWT } from './jose'

Yes I know I'm relying on internals here, but if it breaks, I can quickly fix it by editing one file on my end.

robations commented 3 years ago

In my experience, this can be caused by a missing "main" key in the package.json file, which is see is missing in 3.11.6.

But also, the paths in the documentation don't exist, so I don't see how the import would ever work as documented:

https://unpkg.com/browse/jose@3.11.6/

> var {jwtVerify} = require("jose/jwt/verify");
// Thrown:
{ Error: Cannot find module 'jose/jwt/verify'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18) code: 'MODULE_NOT_FOUND' }

Obviously there is no JS module at node_modules/jose/jwt/verify.js or node_modules/jose/jwt/verify/index.js. Only in the dist directory.

Am I missing something here?

panva commented 3 years ago

In my experience, this can be caused by a missing "main"

There's no "main" entry because there's no "main" package or export. It's just submodules, using a node loader feature present in the supported runtimes. Tooling is slowly catching up, but is just not quite there yet.

so I don't see how the import would ever work as documented

Well, clearly they do - there are plenty of consumers of this package major already. Plus, see the proof of working code i've posted above.

Am I missing something here?

https://nodejs.org/api/packages.html#packages_subpath_exports https://nodejs.org/api/packages.html#packages_conditional_exports

robations commented 3 years ago

Well, clearly they do - there are plenty of consumers of this package

Yes, that's why I thought I was missing something...

Taking a look at the "exports" docs now, I've somehow not come across this before. I'm still stuck on nodejs 10 (and node loader support was added in 12) so I might need to use @tellnes's workaround for now.

Thanks for the quick reply.

panva commented 3 years ago

Well, node 10 is not a supported runtime. Not only because of the module features. But crypto itself requires node 12. LTS ^12.19.0 || ^14.15.0

robations commented 3 years ago

I did notice that (after commenting). What about adding an engines field? Might save you a few unnecessary reports.

https://docs.npmjs.com/cli/v7/configuring-npm/package-json#engines

panva commented 3 years ago

I'm aware of the engines field, but this is a universal package. Not sure how other runtimes would be affected.

robations commented 3 years ago

I see what you mean (eg installing with nodejs, but intended use for browser), but it's only advisory unless you use engine-strict:

Unless the user has set the engine-strict config flag, this field is advisory only and will only produce warnings when your package is installed as a dependency.

And this would allow me to continue using it until I get my stupid projects tested/updated with 12+...

But I only make a suggestion.

panva commented 3 years ago

And this would allow me to continue using it until I get my stupid projects tested/updated with 12+...

Can you elaborate, I'm afraid I don't quite follow.

robations commented 3 years ago

Sorry, ignore that sentence. I thought I had tested the functionality I needed working with nodejs 10, but I was still using 12.

// with nodejs 10
> var {jwtVerify} = require("jose/dist/node/cjs/jwt/verify");
Thrown:
ReferenceError: TextEncoder is not defined

Because of crypto requirements, as you stated.

a7madgamal commented 3 years ago

I have this error in a standard create-react-app typescript setup. I think this should be re-opened and investigated. if we need to take care of something maybe the fix is just adding some guidance on the read.me file.

fnagel commented 3 years ago

Same issue here when using the package with a vue-cli setup:

 ERROR  Failed to compile with 1 error                                                                                                                                                                                          11:30:12

This dependency was not found:

* jose/jwt/verify in ./src/service/license.ts

To install it, you can run: npm install --save jose/jwt/verify
No type errors found
Version: typescript 4.3.2

Using node v14.17.0

panva commented 3 years ago

typescript support for the node's ESM module loader is in the works - https://github.com/microsoft/TypeScript/pull/44501

alanszp commented 3 years ago

To bypass this problem with typescript, I added a alias path to the typescript config:

{
  "compilerOptions": {
    "paths": {
      "jose/jwt/*": ["node_modules/jose/dist/browser/jwt/*"]
    }
  }
}
dalegacusan commented 3 years ago

Any updates on this one? I'm getting the same error with create-react-app typescript.

panva commented 3 years ago

@dalegacusan you're barking up the wrong tree - this is to be solved in your tooling, not here.

panva commented 3 years ago

FWIW create-react-app still uses react-scripts that utilize webpack 4, as soon as a release with webpack 5 (worked on in https://github.com/facebook/create-react-app/issues/9994 and https://github.com/facebook/create-react-app/pull/11201) is done you'll notice "updates".

robcresswell commented 3 years ago

Does anyone have this working with a "current" TypeScript / Node setup (i.e. not yet using ESM, because its somewhat of a struggle with the rest of the TS ecosystem)

I think the only fix I can see is to manually hack around the existing type definitions as in https://github.com/panva/jose/issues/193#issuecomment-831139460

This is mildly disappointing because the library author seems to have made an effort to ship / maintain typedefs, but they're not usable in a Node setup from what I can see so far. Happy to be corrected 😄

panva commented 3 years ago

@robcresswell I really don't understand the struggles here. Not only was I able to make almost anything I wanted work https://github.com/panva/it-should-just-work, but there's TS 4.5 on the horizon with full fledged support now.

robcresswell commented 3 years ago

@panva https://github.com/robcresswell/jose-193 is a minimal repro of what I'm seeing, if that helps

I realise the code itself is nonsense, but the issue is an import path one, not a code one, I think. Also I'm aware there's a good chance I'm being an idiot here, so apologies in advance if I'm wasting your time.

panva commented 3 years ago

@robcresswell all taken from the linked https://github.com/panva/it-should-just-work

diff --git a/main.ts b/main.ts
index 1b7ab92..965dbcc 100644
--- a/main.ts
+++ b/main.ts
@@ -4,6 +4,6 @@ export function main() {
   const encoded = new TextEncoder().encode('');

   new CompactEncrypt(encoded)
-    .setProtectedHeader({ alg: 'RSA-OAEP-256', enc: 'A256GCM' })
-    .encrypt(Buffer.from(''));
+    .setProtectedHeader({ alg: 'dir', enc: 'A256GCM' })
+    .encrypt(Buffer.alloc(32));
 }
diff --git a/package.json b/package.json
index 84a3d6d..22ead9e 100644
--- a/package.json
+++ b/package.json
@@ -4,12 +4,13 @@
   "description": "",
   "main": "index.js",
   "scripts": {
-    "test": "jest"
+    "test": "jest --resolver='./temporary_resolver.js'"
   },
   "author": "",
   "license": "MIT",
   "devDependencies": {
     "@types/jest": "^27.0.2",
+    "enhanced-resolve": "^5.8.3",
     "jest": "^27.2.5",
     "ts-jest": "^27.0.5",
     "typescript": "^4.4.3"
diff --git a/temporary_resolver.js b/temporary_resolver.js
new file mode 100644
index 0000000..eeba8ba
--- /dev/null
+++ b/temporary_resolver.js
@@ -0,0 +1,10 @@
+// temporary workaround until TS 4.5
+
+const resolver = require('enhanced-resolve').create.sync({
+  conditionNames: ['require'],
+  extensions: ['.js', '.json', '.node', '.ts']
+})
+
+module.exports = function (request, options) {
+  return resolver(options.basedir, request)
+}

Your issue is also not a TS one, it's a jest one. npx ts-node main.ts runs just fine, resolves the module and everything. VS Code also properly resolves the types and offers intellisense.


"Jest is a delightful JavaScript Testing Framework with a focus on simplicity."
- https://jestjs.io

🤣

robcresswell commented 3 years ago

@panva Damn, sorry for wasting your time, had tried to be diligent. Thanks for working with me, that was helpful.

panva commented 3 years ago

I'm done waiting for these ecosystems to catch up. jose v4.x will be coming out today/tomorrow with only top level named exports that solve all these tools' problems.

Including webpack@4 stuck create-react-app and out of the box jest