ES6 module is baaadd :(

[uriesk]

Let me take you on a journey....

So i bundle my app with babel and webpack and exclude the external modules from the bundle, like it seems to be very common with node-externals.

Webpack bundles in commonjs per default and with externals, it uses require().

Require obviously doesn't work with ESM modules. So i could either tell webpack to import it as type 'import' and enable dynamic import, which then uses import() for it... or change my app to be a module and... or just bundle it.
Which i decided to do, and made an exception for it and all other passport-x modules.

    externals: [
      nodeExternals({
        // passport-reddit is an ESM module
        // bundle it, then we don't have to import it
        allowlist: [ /^passport-/ ],
      }),
    ],

but oh wait, passport-reddit has a:

import { createRequire } from "module"
const version = createRequire(import.meta.url)('../../package.json').version

which webpack doesn't bundle and then it throws errrors on runtime. So i had to change the import to:

import RedditStrategy from 'passport-reddit/lib/passport-reddit/strategy';

to not include that crap.
None of this pain is neccessary for any other passport strategy.

[Slotos]

Check if use-version-const branch helps.

I'll need more than an appeal to popularity to consider switching to CJS format, however.

[uriesk]

Yes, that solves the weird import at least.

Instead of appeal to popularity, how about providing both esm and cjs? Especially because the other passport strategies are still on commonjs.
Some unrelated packages do stuff like:

  "exports": {
    ".": {
      "import": "./esm/index.js",
      "default": "./cjs/index.js"
    },

So it's possible. It's just uncomfortable to maintain two versions. And too small of a package to justify setting up some bundler to automatically generate those.
It is your choice. I am fine with it now.

[Slotos]

It is possible, but there are security considerations. I need to spend time checking whether any issues described here apply, at the very least. It doesn't look like it would, but I don't want to make the decision without spending some time devising breakage scenarios first.

[gummyWalrus]

Hello there,

I'm trying to implement reddit Oauth 2.0 on my NestJS web app.
A NestJS project is written in typescript and provides a tool '@nestjs/passport' to work with passport strategies easily.
I've successfully implemented Google and Github Oauth 2.0 with this technique.

I don't know as much about NodeJS packages than our buddy uriesk,
But I'm running into a very similar issue where I can't use your module like I did with the other passport-x packages.
Your package being an ES6 Module, it does not fit well in the NestJS typescript environnement and can't be imported in CommonJS modules wich are the best standard that Nest compiles to.

I've tried to change my tsconfig to compile to es6 and went through a LOT of refactoring for it to just break.
You should really consider having both CommonJS and ESM if you want people to use your module in big projects like I tried to do.
Have a nice day

[skeddles]

yeah i dont understand why this module was changed when the other passport modules still use require...

it seems like i cannot simply replace the require statement with import, and it needs to be restructured. require is just so much easier.

at the very least it could be explained with examples on how to convert in the readme.