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.

Sign up for GitHub

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

Jump to bottom

Package not working with typescipt? #242

Closed
gyszalai opened this issue Oct 2, 2023 · 17 comments · Fixed by #291
Closed

Package not working with typescipt? #242

gyszalai opened this issue Oct 2, 2023 · 17 comments · Fixed by #291
Assignees
@mristic505

Comments

@gyszalai
Copy link

gyszalai commented Oct 2, 2023

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 mristic505 self-assigned this Oct 4, 2023
@mristic505
Copy link

@gyszalai can you share tsconfig from your project?

@mristic505
Copy link

@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
Copy link
Author

gyszalai commented Oct 5, 2023

Hi @mristic505! Thank you for your help!

My tsconfig:

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

@gyszalai
Copy link
Author

gyszalai commented Oct 5, 2023

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

@mristic505
Copy link

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
Copy link
Author

gyszalai commented Oct 6, 2023

@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
Copy link

@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
Copy link

mristic505 commented Oct 11, 2023 •
edited
Loading

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
Copy link
Author

@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
Copy link
Contributor

jmjf commented Jun 21, 2024

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
Copy link
Member

simoneb commented Jun 21, 2024

@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
Copy link
Contributor

jmjf commented Jun 21, 2024

@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 simoneb reopened this Jun 21, 2024
@simoneb
Copy link
Member

simoneb commented Jun 21, 2024

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

@jmjf
Copy link
Contributor

jmjf commented Jun 21, 2024 via email

@jmjf jmjf mentioned this issue Jun 22, 2024
Merged
@jmjf
Copy link
Contributor

jmjf commented Jun 23, 2024 via email

@github-actions GitHub Actions
Copy link

🎉 This issue has been resolved in version 9.0.2 🎉

The release is available on:

Your optic bot 📦🚀

@jmjf
Copy link
Contributor

jmjf commented Jun 24, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mristic505 mristic505

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: update types to work with ESM + TS jmjf/get-jwks
4 participants
@simoneb @gyszalai @jmjf @mristic505