Using jwt-decode beta with node10 module resolution about jwt-decode HOT 8 CLOSED

Thanks for sharing.

Not sure it's related, as it does work in node10, it just allows for all kinds of imports and IDE's get confused.

That said, happy to close this yes, thanks for the input!

from jwt-decode.

@jonkoops, just cc'ing you FYI as you were heavily involved in setting this up. I wonder if we can avoid this by not having individual types for both ESM and CJS, as they are identical anyway.

That would ensure we can only do

import { jwtDecode } from 'jwt-decode';
import { jwtDecode as jwtDecode2 } from 'jwt-decode/build/types/index';

which might make more sense (but would need to verify everything still still works).

Will have a look, but I am not sure we need to change anything here. But figured its worth a look.

from jwt-decode.

According to https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md, we should not have a single type declarations file. We have been here before and we went with two different type declarations files for the exact same reason.

I assume there isn't much we can do about this but leave it as is. But curious to your opinion as well @jonkoops before closing again.

from jwt-decode.

Since we don't support any version of Node.js before 18 I don't think this will be an issue. Even though the user could set the node10 module resolution strategy, that code will not run properly in Node.js 16 and up.

The reason for this is that starting with Node.js 16 (AFAIK), it will respect the "exports" field in the package file, which will prevent this kind of access. As per the Node.js documentation:

The "exports" provides a modern alternative to "main" allowing multiple entry points to be defined, conditional entry resolution support between environments, and preventing any other entry points besides those defined in "exports".

So from what I can tell the node10 preset only exists to provide compatibility with historical versions of Node.js that might have these sort of 'illegal' imports going on. Which is fine, as long as they run on older versions that do not respect the "exports" field.

I wonder if we can avoid this by not having individual types for both ESM and CJS, as they are identical anyway.

Like you mentioned, this will then break the node16 resolution strategy, as it needs to have distinct types. I think we should keep them to keep things compatible going forward.

@frederikprijck somewhat related to this, perhaps we should make a clean break and simply roll with the "exports" field only? So essentially remove the "main", "module" and "types" fields. They might still be used, but it could also be the case that most have a modern enough bundler to make things work.

I feel more comfortable shipping without them, rather than pushing them, knowing they might be incorrect. Also adding them back in a quick patch release if we do get issue reports is rather trivial.

from jwt-decode.

Since we don't support any version of Node.js before 18 I don't think this will be an issue. Even though the user could set the node10 module resolution strategy, that code will not run properly in Node.js 16 and up.

Based on my tests, if you run tsc --init, it omits moduleResolution and defaults to node10.

I don't think this will be an issue

It got called out as an issue on another repo. The reporter might eventually end up setting it to node16 but still I was wondering what we can improve.

I feel more comfortable shipping without them, rather than pushing them, knowing they might be incorrect.

We shipped node-auth0 with the same config and havent had any issues yet.

somewhat related to this, perhaps we should make a clean break and simply roll with the "exports" field only? So essentially remove the "main", "module" and "types" fields. They might still be used, but it could also be the case that most have a modern enough bundler to make things work.

I think its still used with a clean tsc project as mentioned above, as that uses node10. But havent had issues with the way its configured. But I am going to give this a try.

from jwt-decode.

Yeah having tested this in a project that gets created using tsc --init, we get the following error when we omit main, module and types properties in the package.json

So as long as node16 isnt the default, I think we should keep it. I agree that node16 is the way to go, TypeScript also says node10 probably shouldnt be used in new projects. Yet it seems to be the default value still.

from jwt-decode.

We shipped node-auth0 with the same config and havent had any issues yet.

If this is battle tested then I feel a little more comfortable shipping like this, it's mostly there to guarantee backwards compatibility. This might be something to re-consider in the future when the platform as a whole has moved on, but if it helps prevent real issues for users let's keep it this way.

To answer your original question, I kind of have to agree that there is no real way of providing a better experience for users of node10. These kind of imports are entirely valid in those versions of Node.js, although the TypeScript compiler perhaps should do a better job at warning users of this potential pitfall.

Should we close this issue then? Considering there is very little we can do.

from jwt-decode.

@frederikprijck I found an issue on the TypeScript project that might be relevant for this discussion: microsoft/TypeScript#55104

from jwt-decode.