Add support for package.json5

[rbong]

Similar to mysticatea/npm-run-all#266

Notes

Removed read-package-json-fast

read-package-json-fast doesn't support json5, and since it is an official NPM tool, it is probably not going to.
Thankfully, everything the package does that we care about is fairly simple.

Mostly, it uses the json-parse-even-better-errors and npm-normalize-package-bin packages.
I am now using these packages directly instead of read-package-json-fast.
None of the things read-package-json-fast does impact npm-run-all2.

Consistency

The return value of the readPackageJson function is still a promise even though the function is now entirely sync.
We could use fs/promise, but nothing else does yet.
This was all for consistency, but let me know if you would like to change it.

Tests

Tests are the same as npm-run-all so I will paste my note from there:

I have tried my best at writing tests for this change.
I don't think it is perfectly in line with the style of your other tests, but this was the simplest way I could see to test it.
If you have other ideas for tests, feel free to give guidance or change my tests when merging.

[socket-security]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/json5@2.2.3	None	0	235 kB	jordanbtucker

🚮 Removed packages: npm/read-package-json-fast@4.0.0

View full report↗︎

[voxpelli]

json5 isn't supported by the package.json ecosystem though?

[rbong]

PNPM does support package.json5 and I have used npm-run-all on projects with it with no issues.
I also personally think it's a great fit, because this allows adding comments to scripts.

If you could clarify more what you mean by the ecosystem and why the ecosystem not supporting it should mean that npm-run-all shouldn't support it, that would be helpful.

[voxpelli]

Eg. Node.js itself doesn’t support reading json5 as far as I’m aware, and there are some properties that it wants to look at

And npm itself parses the json when one publishes a package, and it surely doesn’t support json5?

And any other tool that for some reason wants to find a package.json (to eg identify the closest parent package) will also fail?

[rbong]

You're right, NPM itself does not support JSON5. Other tools will also fail with package.json5.

However, we're not forcing anyone to use JSON5, just using it if it's there.
And since PNPM supports it, there are projects out there which do use it.

It's fairly likely at this point that it breaks something and someone has to use package.json, but why does that something have to be npm-run-all?

Is it just that it's not widely enough supported yet and you don't want the maintenance overhead?
Is there any path forward where you think npm-run-all should reconsider supporting package.json5?

[voxpelli]

I’m mostly worried that someone will then come and request that we support package.yml as well and then package.toml etc

[rbong]

Actually, PNPM supports package.yaml as well. Not sure which is more popular.
Bun supports comments in package.json last I checked as well, technically breaking the format.

All this just to say, I think at least some of the community is dissatisfied with this part of NPM right now, and certain projects will fail with npm-run-all.
In my opinion, the biggest issue is not being able to add comments to scripts (in case you need to document an obscure flag or command you're using to get around some issue), which is why I say it's a great fit.

I understand not wanting to support a ton of new formats, but maybe I can rewrite it in a way that would at least make new formats easier to support, and also make json5 an optional peer dependency?

[bcomnes]

First off thanks for the all the work on this. Going to mull over this if this is a good idea.

In general, secondary ecosystems like yarn and pnpm messing around with adding subtle breaking changes to established node/npm ecosystem design assumptions has been a gigantic time sink for maintainers and consumers with extremely dubious benefits.

Changing a load bearing assumption like package.json parsing as json seems like it is within this territory: jsonc/json5 have had years to establish themselves but have, in general, failed to still gain mass adoption, the argument going: by the time you add comments to json, you might as well use a more robust serialization language like yaml/toml. Also, Crockford says no. That said they have been picking up more pace lately (it seems).

Some questions I can answer for myself with research, but anyone feel free to chime in:

• Comments in package.json: These can't published to npm, so what tool is accommodating them and stripping them? Or are they purely for top-level "projects" that don't get published?
• For tools that do support jsonc/json5, are they using the correct file extension? e.g. package.jsonc/package.json5?
• By supporting comments in .json what will break when people use this capability? What damage will be imposed to the wider tooling ecosystem by allowing this?
• If we were to support package.jsonc/5, what is the full set of additional file extensions that people assume we support (yml/toml)?
• Whats the status of run scripts in pnpm? Are they supporting a pnpm native approach to npm-run-all2 at this point? At which case, would that serve the needs of those interesting in package.json5 comments.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.09%. Comparing base (b8d3ded) to head (cc5d299).
Report is 25 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   95.93%   96.09%   +0.16%     
==========================================
  Files          35       34       -1     
  Lines        2142     2102      -40     
==========================================
- Hits         2055     2020      -35     
+ Misses         87       82       -5     

Flag	Coverage Δ
macos-latest-18	96.09% <100.00%> (+0.25%)	⬆️
macos-latest-lts/*	96.09% <100.00%> (+0.25%)	⬆️
ubuntu-latest-18	95.71% <100.00%> (+0.25%)	⬆️
ubuntu-latest-lts/*	95.71% <100.00%> (+0.25%)	⬆️
windows-latest-18	?
windows-latest-lts/*	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.