minimistjs / minimist

parse argument options
MIT License
515 stars 30 forks source link

Docs: Add sample code ES Modules format to the examples in the README. #61

Closed unikounio closed 4 months ago

unikounio commented 4 months ago

How about adding ES Modules format sample code to the examples in the README?

If you accept my idea, I will create a PR.

My Suggestion

I confirmed it works with Node.js version 20.9.0

CommonJS

// ./example/parse.js
var argv = require('minimist')(process.argv.slice(2));
console.log(argv);
$ node example/parse.js -a beep -b boop
{ _: [], a: 'beep', b: 'boop' }
$ node example/parse.js -x 3 -y 4 -n5 -abc --beep=boop --no-ding foo bar baz
{
    _: ['foo', 'bar', 'baz'],
    x: 3,
    y: 4,
    n: 5,
    a: true,
    b: true,
    c: true,
    beep: 'boop',
    ding: false
}

ES Modules

// ./example/parse.mjs
import minimist from "minimist";
const argv = minimist(process.argv.slice(2));
console.log(argv);
$ node example/parse.mjs -a beep -b boop
{ _: [], a: 'beep', b: 'boop' }
$ node example/parse.mjs -x 3 -y 4 -n5 -abc --beep=boop --no-ding foo bar baz
{
    _: ['foo', 'bar', 'baz'],
    x: 3,
    y: 4,
    n: 5,
    a: true,
    b: true,
    c: true,
    beep: 'boop',
    ding: false
}
shadowspawn commented 4 months ago

Ok from me adding explicit cjs/esm. There is essentially only one line different though, so I think there could be cjs/esm example code but there does not need to be two versions of the example calls. And to make it more obvious the only different is the import, I would put the require/import on the first line so the code is the same for the other two lines. (I feel putting extra code on the same line as the require is "clever" and compact, but clearer to do require on a line on its own.)

Wait for comment from @ljharb too (other maintainer).

(I assume we can't easily do the cjs/esm tab thing like on the nodes.org web site.)

ljharb commented 4 months ago

I guess we can include both, sure, but knowing how CJS works is something beginners need to learn anyways.

No, we can’t do tabs like that.

Doing the require on the same line is good to show because that’s a feature ESM lacks.

However we do it, I’d only want to see the two additional import lines, and nothing else duplicated.

unikounio commented 4 months ago

Thank you both for your prompt feedback. Based on your suggestions, I'm thinking of proceeding by placing the require/import line at the beginning, to clearly demonstrate that the rest of the code is common between both formats.

Feedback-Based Suggestion

//for CommonJS
// ./example/parse.js
const argv = require('minimist')(process.argv.slice(2));

//for ES Modules
// ./example/parse.mjs
import minimist from 'minimist';
const argv = minimist(process.argv.slice(2));

//Common
console.log(argv);
$ node example/parse.js -a beep -b boop
{ _: [], a: 'beep', b: 'boop' }
$ node example/parse.js -x 3 -y 4 -n5 -abc --beep=boop --no-ding foo bar baz
{
    _: ['foo', 'bar', 'baz'],
    x: 3,
    y: 4,
    n: 5,
    a: true,
    b: true,
    c: true,
    beep: 'boop',
    ding: false
}
shadowspawn commented 4 months ago

Lots of comments, I think a lot about READMEs, sorry. 😅

I like the idea of having example files for both, which is implied by your description. Of course! I was forgetting we had an explicit matching example file, small as it is.

I think having both imports and the //Common in same code block makes the code not directly usable and a little clunky to read. Maybe just have one commented out?

For interest, a couple of patterns I have used elsewhere.

1) I add explicit links to the corresponding example file from the README. Besides linking to file, also introduces the filename which gets used in the example call.

2) The example files use an import from the repo so they can be run from the repo, so I add a "normal" import in a comment too so there is not just the "local" import.

Trying out some of those ideas, see what you think.


example

Example files: example/parse.js (CommonJS) example/parse.mjs (ECMAScript)

const minimist = require('minimist'); // (for CommonJS)
// import minimist from 'minimist'; // (for ECMAScript)

const argv = minimist(process.argv.slice(2));
console.log(argv);
unikounio commented 4 months ago

Thanks for the great ideas! It is very clear when include those ideas. I will also add example/parse.mjs and send you a PR.😄

unikounio commented 4 months ago

This issue will be closed as the related PRs have been successfully merged. Thank you to everyone who contributed, provided feedback, and supported this effort. Your cooperation has been invaluable.