nearform / get-jwks

Fetch utils for JWKS keys
MIT License
25 stars 14 forks source link

Package not working with typescipt? #242

Closed gyszalai closed 1 week ago

gyszalai commented 9 months ago

Hi all!

I'm trying to use this package from typescript like this:

import buildGetJwks from 'get-jwks'
const getJwks = buildGetJwks()

and when I try to compile the typescript code, I get the following error:

src/my-code.ts:7:21 - error TS2349: This expression is not callable.
  Type 'typeof import(".../node_modules/.pnpm/get-jwks@8.3.1/node_modules/get-jwks/src/get-jwks")' has no call signatures.

7     const getJwks = buildGetJwks()

node.js: 20.8.0 and 18.17.1 typescript: 5.2.2 pnpm: 8.8.0

Am I doing something wrong or is this module not compatible with either node 18/20 or typescript 5.2?

Please help me! Thank you in advance!

mristic505 commented 9 months ago

@gyszalai can you share tsconfig from your project?

mristic505 commented 9 months ago

@gyszalai I was able to confirm that it works with:

node.js: 18.17.1 typescript: 5.2.2 pnpm: 8.8.0

The issue is somewhere on the application level, very likely in your tsconfig settings.

gyszalai commented 9 months ago

Hi @mristic505! Thank you for your help!

My tsconfig:

{
    "extends": "fastify-tsconfig",
    "compilerOptions": {
        "outDir": "dist",
        "sourceMap": true
    },
    "include": ["src/**/*.ts"]
}
gyszalai commented 9 months ago

I created my tsconfig according to the documentation of fastify-tsconfig: https://github.com/fastify/tsconfig

mristic505 commented 9 months ago

Hi @mristic505! Thank you for your help!

My tsconfig:

{
    "extends": "fastify-tsconfig",
    "compilerOptions": {
        "outDir": "dist",
        "sourceMap": true
    },
    "include": ["src/**/*.ts"]
}

@gyszalai

I am able to confirm that get-jwks works with those simple tsconfig settings that you provided and with node 18.17.1, typescript: 5.2.2 and pnpm: 8.8.0. Did you try to create a simple "Hello World" with a single index.ts file example to see if it successfully compiles? If you are able to make it work that way then you are very likely experiencing some other application level issues that are unrelated to get-jwks.

gyszalai commented 9 months ago

@mristic505 Thanks for your efforts! Just created a small TS project with only one index.ts and I could reproduce the problem. You can find the project here: https://github.com/gyszalai/get-jwks-ts-issue

mristic505 commented 8 months ago

@gyszalai

Thank you for replicating the issue.

The problem disappears once you remove "type": "module" from package.json. If you are using this option only to enable ECMAScript import/export syntax, it will still work in your ecosystem with this option removed. In addition to this, I recommend using Node 18.18.0 since it is LTS.

mristic505 commented 8 months ago

Closing this issue due to our conclusion that it is not due to the package but due to application settings in the provided example. It works when using both require and import statements.

We are able to confirm that the package works with typescript.

Please open a new issue if you keep experiencing problems and you still think that it is due toget-jwks.

gyszalai commented 8 months ago

@gyszalai

Thank you for replicating the issue.

The problem disappears once you remove "type": "module" from package.json. If you are using this option only to enable ECMAScript import/export syntax, it will still work in your ecosystem with this option removed. In addition to this, I recommend using Node 18.18.0 since it is LTS.

@mristic505 thank you for your help! btw, we use node 20 because it is a new app and node 20 will be LTS from October 24th.

jmjf commented 1 week ago

For those who can't easily remove "type": "module" in package.json:

import buildGetJwks, { type GetJwksOptions, type GetJwks} from 'get-jwks`

type BuildGetJwksType = (options?: GetJwksOptions) => GetJwks

// type casting incantation avoids errors when using ESM
const getJwks = (buildGetJwks as unknown as BuildGetJwksType)({...})

It seems the issue is that TypeScript can't figure out the type of buildGetJwks when ESM is active. I guess the type declaration in the get-jwks.d.ts (reference below) is ignored on import buildGetJwks from 'get-jwks'.

get-jwks.d.ts: https://github.com/nearform/get-jwks/blob/main/src/get-jwks.d.ts -- if interface changes, BuildGetJwksType above should align with the type for buildGetJwks in get-jwks.d.ts.

If you call buildGetJwks several times, you may prefer to do something like const typedBuildGetJwks = buildGetJwks as unknown as (options?: GetJwksOptions) => GetJwks and use typedBuildGetJwks to avoid the type incantation on every call. Optionally put it in a module, export it, and then import typedBuildGetJwks wherever you need it.

simoneb commented 1 week ago

@jmjf our earlier investigation suggested that this wasn't an issue with the package, but with the way it was being used. If you think it's instead an issue with the package and the typings, I'm happy to reopen the issue.

jmjf commented 1 week ago

@simoneb

After a bike ride :), I tried this in the work project where I ran into this issue. That project is set up to use ESM, so "type": "modules", a ts-node/esm loader, etc.

I created two identical files, one named getJwks.mjs and one named getJwks.ts.

// getJwks.mjs and getJwks.ts
import buildGetJwks from 'get-jwks';
export const getJwks = buildGetJwks();

I ran each file with node --import tsnode.mjs, where tsnode.mjs loads ts-node/esm (avoids warnings about --loader; Node 20.8+ required).

// tsnode.mjs
import {register} from 'node:module'
import {pathToFileURL} from 'node:url'

register('ts-node/esm', pathToFileURL('./'))

When I run getJwks.mjs, it runs. When I run getJwks.ts it fails.

I think that says this is a TypeScript problem or an ESM + TypeScript problem.

So I thought, "Well, @fastify/jwt uses a default import and I get no problems from that. How are they doing it?" answer

So I hacked get-jwks.d.ts in node_modules, replacing the export default... with:

declare function buildGetJwks(options?: GetJwksOptions): GetJwks
export = buildGetJwks

Now both ts and mjs versions work. I didn't test CJS because ts-node/esm doesn't like CJS, so probably worth checking that this style of export doesn't break. OTOH, it works for @fastify/jwt and CJS, so will probably be fine.

simoneb commented 1 week ago

@jmjf would you be so kind as to send a PR too? You've done all the heavy lifting already it seems :)

jmjf commented 1 week ago

Sure. Will set that up this weekend. 🤗

On Fri, Jun 21, 2024, 12:32 Simone Busoli @.***> wrote:

@jmjf https://github.com/jmjf would you be so kind as to send a PR too? You've done all the heavy lifting already it seems :)

— Reply to this email directly, view it on GitHub https://github.com/nearform/get-jwks/issues/242#issuecomment-2183071733, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASIV6ZYT66G4ZDSUGUVQFLZIRIQ3AVCNFSM6AAAAABJV6J74WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBTGA3TCNZTGM . You are receiving this because you were mentioned.Message ID: @.***>

jmjf commented 1 week ago

And I just saw this in Node Weekly.

https://arethetypeswrong.github.io/?p=get-jwks%409.0.1

Which reports the problem the PR should fix.

On Fri, Jun 21, 2024, 12:39 Jamee Mikell @.***> wrote:

Sure. Will set that up this weekend. 🤗

On Fri, Jun 21, 2024, 12:32 Simone Busoli @.***> wrote:

@jmjf https://github.com/jmjf would you be so kind as to send a PR too? You've done all the heavy lifting already it seems :)

— Reply to this email directly, view it on GitHub https://github.com/nearform/get-jwks/issues/242#issuecomment-2183071733, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASIV6ZYT66G4ZDSUGUVQFLZIRIQ3AVCNFSM6AAAAABJV6J74WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBTGA3TCNZTGM . You are receiving this because you were mentioned.Message ID: @.***>

github-actions[bot] commented 1 week ago

🎉 This issue has been resolved in version 9.0.2 🎉

The release is available on:

jmjf commented 1 week ago

And arethetypeswrong agrees. 🥳

On Mon, Jun 24, 2024, 11:25 github-actions[bot] @.***> wrote:

🎉 This issue has been resolved in version 9.0.2 🎉

The release is available on:

Your optic https://github.com/nearform-actions/optic-release-automation-action bot 📦🚀

— Reply to this email directly, view it on GitHub https://github.com/nearform/get-jwks/issues/242#issuecomment-2186840123, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASIV664MHHE7D357TEG7QLZJA25VAVCNFSM6AAAAABJV6J74WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBWHA2DAMJSGM . You are receiving this because you were mentioned.Message ID: @.***>