feat: switch to explicit type: module

[brettz9]

feat: switch to explicit type: module or type: commonjs

This is recommended because Node states:

Writing ES module syntax in "ambiguous" files incurs a performance cost, and therefore it is encouraged that authors be explicit wherever possible. In particular, package authors should always include the "type" field in their package.json files, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes, and it will also make things easier for build tools and loaders to determine how the files in the package should be interpreted.

This necessitated supplying type: commonjs to dist directories so that imports wouldn't now err, and so that older browsers wouldn't break with a .cjs extension content type issue or with consumers needing to update changed paths.

Also:

• refactor: uses esm in binary files (except the /acorn/bin/acorn one used in production as it would break engines) and in test files; I kept the benchmark package as commonjs in case you wanted to allow benchmarking in older browsers
• docs: uses esm in examples (except where describing CJS)
• chore: adds package-lock.json files (they are not excluded from version control)

I noticed that if the Unicode regular expression script is run, more scripts will be added, but I did not include that here, as it appears acorn/src/unicode-property-data.js still needs updating.

[RReverser]

This shouldn't be necessary because we have exports fields which is a newer, more flexible, mechanism and supersedes type: "module". Do you see a warning from Node.js that suggests otherwise? (when doing the double-parsing thing, it always shows a warning in the console)

[brettz9]

This shouldn't be necessary because we have exports fields which is a newer, more flexible, mechanism and supersedes type: "module". Do you see a warning from Node.js that suggests otherwise? (when doing the double-parsing thing, it always shows a warning in the console)

In regard to "newer, more flexible mechanism", I think you are thinking of the module property which indeed was non-standard and is no longer recommended (which is why this PR removes it). The type property is still recommended (in conjunction with exports), however, as per the current Node docs as cited in the OP.

There are no warnings showing up; it is merely for best practices.

[RReverser]

I think you are thinking of the module property which indeed was non-standard

No, I'm not, that one is even older.

There are no warnings showing up; it is merely for best practices.

In that case the resolution already picks up the correct export. type: "module" is used only if exports is not available - it's for simple cases where you only have one central file anyway and don't support multiple import mechanisms. Whenever module supports multiple formats, exports is preferable and type will be ignored.

[RReverser]

As per exports docs:

It is supported in Node.js 12+ as an alternative to the "main" that can support defining subpath exports and conditional exports while encapsulating internal unexported modules.

Basically, if you have a single format, you use type + main fields. If you have multiple formats in the same package, you use the more flexible exports instead.

[brettz9]

As per exports docs:

It is supported in Node.js 12+ as an alternative to the "main" that can support defining subpath exports and conditional exports while encapsulating internal unexported modules.

Basically, if you have a single format, you use type + main fields. If you have multiple formats in the same package, you use the more flexible exports instead.

That is an alternative to main, not an alternative to type. type is still, as per the quotation in the OP, still recommended.

[RReverser]

It's both because type applies only if you don't have exports.

As I said, type is for when your package has a single type - ESM or CommonJS. This is not the case for our packages, hence using exports instead.

It doesn't make sense to add type because neither format is prioritised over the other in packages like ours.

[RReverser]

I appreciate the contribution, but I don't think this particular change is worth it.

That said, I'd be happy to accept a trimmed-down PR with just the CommonJS->ESM changes in our scripts, as that makes simplifies importing test helpers in browser and Node.

[brettz9]

It turns out I was mistaken. There are warnings that show up. When I run the tests or binary files as imports I get this:

(node:46698) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///Users/brett/acorn/bin/generate-identifier-regex.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to /Users/brett/acorn/package.json.

[brettz9]

If I convert all the new ESM files to .mjs files, there shouldn't be any more warnings, however.

[marijnh]

Writing ES module syntax in "ambiguous" files incurs a performance cost

This is simply not happening in this package. Client code imports the dist/ files resolved via the exports in the package.json file, and gets the appropriate format right away.

The warning you quote happens because you changed this file to be ESM. It's not an issue in the current code.

chore: adds package-lock.json files (they are not excluded from version control)

We have a package lock at the top level. I don't see any reason to add locks to the individual packages.

Changing our sources and build system to ESM might be a good idea, but that can be done without changing anything about the way the distributed code works.

Similarly, we could use ESM for our utility scripts and use type: "module" to make that the default for .js files in the package, but if that also affects bin/acorn I'd want to be sure that that change doesn't affect any users.

A big PR like this that conflates a bunch of issues and makes unrelated changes is not something that's welcome, sorry.