ES Modules broke the CLI?

[kemitchell]

Invoking the CLI since 0.29.0 results in a runtime error due to the #!/usr/bin/env -S node -r esm shebang. I've reproduced the issues installing globally with npm i -g and installing in a project's node_modules for invocation via via node_modules/.bin/commonmark or npx commonmark.

Relevant Commit: 9bcf45b

I am not up-to-date on Node.js and ESM support, or this package's Node.js version compatibility contract.

I will poke around a bit now and try to see if I can introduce a test for CLI invocation.

$ node -v
v14.15.1
$ npm i -g https://github.com/commonmark/commonmark.js
# 6d80c8eedd09189f089568710dcee5c964dedb37 as of now
[...]
$ commonmark
internal/modules/cjs/loader.js:883
  throw err;
  ^

Error: Cannot find module 'esm'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1217:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:447:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:75:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}
$ npm i -g esm
[...]
$ commonmark
internal/modules/cjs/loader.js:883
  throw err;
  ^

Error: Cannot find module 'esm'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at Module._preloadModules (internal/modules/cjs/loader.js:1217:12)
    at loadPreloadModules (internal/bootstrap/pre_execution.js:447:5)
    at prepareMainThreadExecution (internal/bootstrap/pre_execution.js:75:3)
    at internal/main/run_main_module.js:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}

[jgm]

Actually I don't know if this is the right fix.
Requiring esm may have bad consequences for people who use this as a library (the most common use).
Maybe someone who knows a little about JS can comment.

[kemitchell]

IIRC, the esm package is a loader for ES Modules on older version of Node >= 6 that don't support them out of the box. More recent versions do.

Does this package currently make any kind of Node.js version compatibility guarantee?

[jgm]

Does this package currently make any kind of Node.js version compatibility guarantee?

No, I don't think so.
Which versions (and how recent?) support them out of the box?

[kemitchell]

IIRC, one of the 13.x releases enabled ESM by default. Prior releases had it behind a flag, like --experimental-modules.

[jgm]

I guess it's tempting to revert this change and just let bin/commonmark fail if people use older node versions.
That way we don't burden library users with an extra, unnecessary dependency.
Or is there a way to say in package.json: require this library unless you have such and such node version?
Alternatively, we could require a recent version of node, but I hesitate to do that as it may break things for some people.

[kemitchell]

@jgm, by the by, I am also having problems using this package as a library from ES Modules of my own since 0.29. import commonmark from 'commonmark' fails with a message about trying to use import with a CommonJS module.

lib/index.js is indeed an ES Module, but "type":"commonjs" is set in package.json at the root, so Node treats the .js extension as CommonJS. Apparently npm run build also builds dist/commonmark.js and dist/commonmark.min.js, and those end up in the npm tarballs, too.

What was the original impetus for shifting to an ES Module? We have unflagged support for those in newer versions of Node.js, but Node.js' implementation allows using the new import syntax with old-style modules, no problem. Just based on my own experience, I also get the feeling folks coding for the browser are still largely bundling with tools like rollup. That leaves us with:

• If you don't care about old versions of Node.js, you can set "type":"module" in package.json or export modules with the .mjs extension.

• If you do care about old versions of Node.js, you can just stick with require and CommonJS. Folks who want to use the new import syntax can do that. Bundlers can bundle the code just fine.

• The worst we can do is confuse Node.js and bundlers about whether the code is CommonJS or ES Modules. That's what's happening with the ambiguous .js file extension and "type":"commonjs" in package.json in 0.29.

[jgm]

See #136, #182 and #186 for the original issue and PR.
The original motivation was to be able to use rollup and webpack tree shaking. But I wanted to retain backwards compatibility, and didn't want to require recent node.js.

If you do care about old versions of Node.js, you can just stick with require and CommonJS. Folks who want to use the new import syntax can do that. Bundlers can bundle the code just fine.

This would amount to reverting the changes in #182/#186. I was led to understand that this would mean the bundlers can't do dead code elimination. But maybe that's okay. I hadn't realized the change would have so many bad consequences. (And obviously, I really don't know what I'm doing when it comes to the JS ecosystem; I rely on advice here.)

[kemitchell]

Tree shaking is a thing, though not exclusively in ESM. CJS build systems do it, too. In any event, I'm not sure how much tree shaking actually happens with the external dependencies of this package. They all seem to be brought in with import *. But there's nothing the matter with writing newer-style JavaScript.

Two thoughts come to mind.

First, this package is still in 0.x.y. It's also broadly deployed, but folks are on notice to look out for significant changes. Maintainer has some leeway.

Second, insofar as people want bundled or minified or otherwise processed distribution copies of this code, there are oodles of free tools out there to do the job, and they're probably using some on their own app code, anyway. Requirements and preferences vary widely.

I don't know how many folks are getting this through Bower. I don't know how many are relying on minified copies from dist. But those are just two examples of many particular ways to "package" the core work here.

I can say package.json has a lot of cruft in it, some of which is causing problems. I will make some time to look at it again tomorrow if I can.

[jgm]

Thank you. Advice will be welcome.