apostrophecms / random-words

Generate one or more common English words. Intended for use as sample text, for example generating random blog posts for testing
MIT License
249 stars 72 forks source link

Export CommonJS version #43

Open ologbonowiwi opened 11 months ago

ologbonowiwi commented 11 months ago

The problem to solve

Is your feature request related to a problem you are experiencing that is not a bug? Please describe.

I want to use the project, but the project I'm working on does not support ESModules.

Proposed solution

It'd be nice to have a /commonjs counterpart to the current implementation.

Alternatives

Use something like babel to output a CommonJS version before publishing it

boutell commented 11 months ago

In any async function in commonjs code in Node.js, you can write:

const { generate, count } = await import('random-words');

We plan to use this method ourselves.

I'm interested to know why and whether your project can't use this technique.

ologbonowiwi commented 11 months ago

It's a legacy project that has +2 years; I'm enhancing the testing coverage, hoping to have more reliability for further changes.

The thing is that this project is running on CommonJS in an older Node version (before top-level await); my wish was to use the project to generate some params for a jest it.each. All tests in Jest must be declared synchronously, so having it inside an async function would not be an option.

I ended up installing @faker-js/faker on the end :confused:, but having this one portable to CommonJS would be helpful, and I'm sure it'd enhance the project's usage.

ologbonowiwi commented 11 months ago

The most significant benefit that I see on this project and that faker does not support is setting the seed. Since for automated testing, it'd be pretty beneficial to have consistent randomness, so the test executed would always be the same (I'm using the generated result to declare the test name)

boutell commented 11 months ago

I think the versions of node you’re referring to are not supported upstream, so it’s probably not safe to be using them in any case. Node 18 is the oldest supported release.

On Fri, Dec 8, 2023 at 10:32 AM Wesley @.***> wrote:

The most significant benefit that I see on this project and that faker does not support is setting the seed. Since for automated testing, it'd be pretty beneficial to have consistent randomness, so the test executed would always be the same (I'm using the generated result to declare the test name)

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/random-words/issues/43#issuecomment-1847390031, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27KITR7SNMJKZ7SOVWDYIMXJTAVCNFSM6AAAAABAMVHY6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBXGM4TAMBTGE . You are receiving this because you commented.Message ID: @.***>

ologbonowiwi commented 11 months ago

We're under v18.14.2 here. I'll close this, and when I have some availability, I can fork it and add CommonJS support.

Thank you anyway!

boutell commented 11 months ago

FYI, Node 18 supports top level await. Only in .mjs files, and that's not going to change, but you could do your dynamic import in an async function. I think it might make more sense though for you to do your own babelization rather than an actual fork so you don't have to maintain the code. Best wishes with your project.

On Fri, Dec 8, 2023 at 11:16 AM Wesley @.***> wrote:

We're under v18.14.2 here. I'll close this, and when I have some availability, I can fork it and add CommonJS support.

Thank you anyway!

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/random-words/issues/43#issuecomment-1847467060, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27MVI4CPFP7PO4DIQ7TYIM4PPAVCNFSM6AAAAABAMVHY6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBXGQ3DOMBWGA . You are receiving this because you commented.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

AlanGreyjoy commented 10 months ago

Why are we trying to normalize esm on a backend......

pksunkara commented 10 months ago

I am compiling TS to code to CommonJS to run on Node 18 and none of the approaches are working:

import { generate } from 'random-words';
async function slugify() => {
    // ...
    const { generate } = await import('random-words');
    // ...
}

Both of them error out with ERR_REQUIRE_ESM

ologbonowiwi commented 10 months ago

@pksunkara it's returning this error because since you're transpilling into common js, you don't run it in ESM.

About the second version, the exported code uses await import or it's being changed to other thing?

pksunkara commented 10 months ago

Ofcourse I understand what the issue is.

All I am trying to say is that because of how tsc is compiling the second version and changing it, even the second version doesn't work.

Basically the conclusion is that this library does not work at all for commonjs backend with typescript.

boutell commented 10 months ago

Is this an issue somehow unique to the library or are you having a general problem using ES modules in your specific environment? If the latter I suggest taking that to stackoverflow or another appropriate forum and creating simple test case ES modules to experiment with as it is not something specific to this library.

On Tue, Jan 9, 2024 at 7:23 AM Pavan Kumar Sunkara @.***> wrote:

Ofcourse I understand what the issue is.

All I am trying to say is that because of how tsc is compiling the second version and changing it, even the second version doesn't work.

Basically the conclusion is that this library does not work at all for commonjs backend with typescript.

— Reply to this email directly, view it on GitHub https://github.com/apostrophecms/random-words/issues/43#issuecomment-1882976982, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27JQYWMJGO4MEGEKPHDYNUZDPAVCNFSM6AAAAABAMVHY6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBSHE3TMOJYGI . You are receiving this because you commented.Message ID: @.***>

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

max-degterev commented 9 months ago

For everyone looking for CSJ alternatives: https://www.npmjs.com/package/random-word-slugs

It is the same issue as with all Sindre Sorhus packages: legacy codebases exist. Splitting ecosystem into opposing camps doesn't help anyone. Add a compile step people.

boutell commented 9 months ago

There's no intention here to split the ecosystem, we just can't support everything and have chosen to offer an ES module. For free, keep in mind. Use what you like, and consider upgrading your tooling to something that can also handle ES modules. Or use something else; that's fine too.

anton-series commented 1 month ago

I spent a day trying to make this package work with ts-node, Node 20 and TypeScript, but I gave up.

Instead, here's a forked version that supports both CommonJS and ESM: https://www.npmjs.com/package/random-words-commonjs

Changes: https://github.com/anton-series/random-words-commonjs/pull/1

boutell commented 1 month ago

All right, I see your point.

boutell commented 1 month ago

At the time I declined to support this, I had not realized the extent to which it was painful to use from commonjs. Since then I've had occasion to actually try that, with other modules, and you're right: it's a bad scene.

This is actually scheduled to change - node 22 will, at some point, start supporting requires of ES modules as long as they have no top level async code - but in the meantime, it makes more sense to have tooling for it.

Would you like to submit your changes as a PR? As long as the update procedure is clear I see the benefit at this point. Sorry for making this contentious.

pksunkara commented 1 month ago

FWIW, with even with that support in node 22, ts-node still doesn't work. Ref: https://github.com/TypeStrong/ts-node/issues/1007#issuecomment-2181478441

boutell commented 1 month ago

Yeah, I'm not surprised. I hate having so much tooling required for everything, but it's the way of the world.