Add support for node hybrid cjs and esm modules

[perrin4869]

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

Adds support for loading this module natively as esm from node.js.
This also makes it possible to consume this module with the new nodenext and node16 typescript module resolvers.
This is not possible in the current state of affairs, because the .d.ts file is written for esm modules while typescript will attempt to load the commonjs version of the package.

References

https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/
https://nodejs.org/api/packages.html#exports

[perrin4869]

@frederikprijck Could you check now if things work better?
This is somewhat tricky to setup 😅

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you have not received a response for our team (apologies for the delay) and this is still a blocker, please reply with additional information or just a ping. Thank you for your contribution! 🙇‍♂️

[perrin4869]

I'm still interested in this PR just in case 😄

[jonkoops]

Another idea, now that ESM is widely supported in all versions of Node.js. Why not drop CommonJS compatibility and remove the build step entirely?

[perrin4869]

I'm all for it, if that's the direction @frederikprijck agrees to take this module in I'll update this PR

[remcohaszing]

This field should match the main field.

Suggested change

	"types": "index.d.mts",
	"types": "index.d.ts",

[remcohaszing]

The default export should match the type field in package.json. The absence of type, means it’s commonjs.

Also the value that matches the default can be omitted.

Suggested change

	"types": {
	"require": "./index.d.ts",
	"import": "./index.d.mts",
	"default": "./index.d.mts"
	},
	"require": "./build/jwt-decode.cjs.js",
	"import": "./build/jwt-decode.mjs",
	"default": "./build/jwt-decode.mjs"
	"types": {
	"import": "./index.d.mts",
	"default": "./index.d.ts"
	},
	"import": "./build/jwt-decode.mjs",
	"default": "./build/jwt-decode.cjs.js"

[remcohaszing]

I was going to make a PR to fix the type definitions. Then I found this PR goes a step further by adding native dual publishing.

Thank you for this great work @perrin4869!

Some additional context: TypeScript 4.7 added the "module": "node16" option. This corrects a lot of issues with type definitions for both native ESM and dual publishing. The type definitions were always wrong, but this has become more apparent now. According to the type definitions without this PR, this is correct usage:

CJS (JavaScript)

const jwtDecode = require('jwt-decode');

jwtDecode.default(token);

CJS (TypeScript)

import jwtDecode = require('jwt-decode');

jwtDecode.default(token);

ESM (same syntax for JavaScript and TypeScript)

import jwtDecode from 'jwt-decode';

jwtDecode.default(token);

Without my suggestions, these issues still exist for TypeScript’s "moduleResolution": "node" option.

[perrin4869]

@remcohaszing Thanks for the review!! That was really useful! Updated according to your suggestions 😃

[bertybot]

Wouldn't we need the "type": "module" field set in the package too? That's what is causing problems with my esm set up. Also is there any update on this or at least work arounds for now? I wish we would just kill CJS Lol

[cristobal]

Would it not be better to have the default as type:module for the jwt-decode.js and rather have an .cjs export file instead.

[cristobal]

Any updates or thoughts on this PR @frederikprijck?

Also as stated by @jonkoops 👇🏽

Another idea, now that ESM is widely supported in all versions of Node.js. Why not drop CommonJS compatibility and remove the build step entirely?

Why still support for AMD?

AMD is more or less dead, so the thought of having the standalone which takes into account AMD and fallbacks to windows makes no sense, most people can polyfill this themselves a section in the docs would suffice here imo.

Another Approach

Another approach here could have been:

1. Drop the standalone export
2. Single Typescript definition (also perhaps as an ambient module)
3. Use ESM type: module all the way
4. Have a fallback cjs. export if only it's required.

[frederikprijck]

@cristobal

Another idea, now that ESM is widely supported in all versions of Node.js. Why not drop CommonJS compatibility and remove the build step entirely?

There are no plans to do this. Other libraries have done this before, and that hasn;t been appreciated, see node-fetch/node-fetch#1263 as an example.

Additionally, this is not a node library, but a browser library. So the support of node for ESM isn't what should drive our decision. This library works on node, but wasn't created with node in mind (as mentioned in the readme).

As I know this library is used in node, we have no plans to drop support for that (and I am happy to improve the support by using node's atob instead of our own polyfill now that node 14 is no longer supported). Just know that it's not the main reason of its existence.

That's ofcourse not to say that browsers can't support ESM. Regardless, we have improved all of this in #151 . Feel free to provide feedback on that PR.

Given the potential breaking change of this PR, we didn't want to ship the PR as is but include it in our beta and implement it slightly differently (but pretty comparable). Regardless, appreciate the PR and work here @perrin4869.

AMD is more or less dead, so the thought of having the standalone which takes into account AMD and fallbacks to windows makes no sense, most people can polyfill this themselves a section in the docs would suffice here imo.

I mean, I considered it. But for a library that's downloaded 5 millions times a week, I see no reason to drop support for something that isn't causing issues. I mean, the download numbers shouldnt drive our decisions, but it's hard to know who still uses AMD or who doesn't and we see not strong need to drop AMD. However, we did simplified the standalone file and rely on rollup to handle the UMD bundle with AMD.

Closing this PR as we should have covered that in that branch.