Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
unikounio opened this issue Mar 9, 2024 · 6 comments
Closed

Comments

@unikounio
Copy link
Contributor

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

  • Beginners might be confused when there are no sample codes in the ES Modules format.

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
Copy link
Collaborator

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
Copy link
Member

ljharb commented Mar 9, 2024

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
Copy link
Contributor Author

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
Copy link
Collaborator

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.
  1. 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

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

No branches or pull requests

3 participants