Skip to content

refactor: revert to common js#278

Closed
123FLO321 wants to merge 2 commits intosimllll:masterfrom
123FLO321:br_cjs
Closed

refactor: revert to common js#278
123FLO321 wants to merge 2 commits intosimllll:masterfrom
123FLO321:br_cjs

Conversation

@123FLO321
Copy link
Copy Markdown
Contributor

Motivation:

CJS is still dominant in the Node echosystem and is still the default.
Since there are no benefits to ESM in this project that i can think i don't think it's a good idea to switch to ESM (yet).
ESM makes it difficult (and i belive even impossible) to use this module in CJS projects.
Making this ESM only makes it inpossible (as far as i know) to use this in an NestJS project.

@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 11, 2022

I've created #281 and will release it right now, please check if it works for you.
It shuld also work in a nextJs project, in case you still have troubles, maybe you can share a basic setup with nestjs so we can see how to fix that issue too

@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 12, 2022

Just released another version, because the "radius" package import was broken. So far eveerything works now on my side, please let me know if it works for you too

@123FLO321
Copy link
Copy Markdown
Contributor Author

Still doesn't allow me to use it in a Nest project with version 2.1.4

Bootstrap Failed: Error [ERR_REQUIRE_ESM]: require() of ES Module node_modules\radius-server\dist\index.js from dist\src\network\network.service.js not supported.
Instead change the require of index.js in dist\src\network\network.service.js to a dynamic import() which is available in all CommonJS modules.

Import is done like this

const { RadiusServer } = await import("radius-server");

But get's compiled to this using the TypeScript compiler

const { RadiusServer } = await Promise.resolve().then(() => __importStar(require("radius-server")));

@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 13, 2022

Are we talking about nest js? https://github.com/nestjs/nest/releases There are quite newer versions than 2.1.4?

@123FLO321
Copy link
Copy Markdown
Contributor Author

Version 2.1.4 of this repo https://github.com/simllll/node-radius-server/releases/tag/v2.1.4.
Latest Nest version.

@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 14, 2022

How do you transpile it? via tsc? what is your tsconfig? Check if there any tickets regarding this in nestjs.. I'm pretty sure people are using esm modules with nest js or it should at lesat be possible to do so nowadays?

@123FLO321
Copy link
Copy Markdown
Contributor Author

According to StackOverflow there is currently no way to import esm modules in cjs typescript except by using an eval statement https://stackoverflow.com/questions/70545129/compile-a-package-that-depends-on-esm-only-library-into-a-commonjs-package/70546326#70546326

@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 14, 2022

Can you try this setting in your tsconfig:

{
  "compilerOptions": {
    ...
    "module": "node12",
    ...
  },
...
}

@123FLO321
Copy link
Copy Markdown
Contributor Author

That works but requires an unstable typescript version (@next)

@123FLO321
Copy link
Copy Markdown
Contributor Author

The following code works in a nest typescript project.
It's really ugly and stupid but works.

const { RadiusServer } = await (eval(`import("radius-server")`) as Promise<typeof import("radius-server")>);

@123FLO321 123FLO321 closed this Apr 14, 2022
@simllll
Copy link
Copy Markdown
Owner

simllll commented Apr 19, 2022

There is also ts 4.7 coming: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7-beta/ which has then the node12 module :-)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants