Prefer package.json5 over package.json

[JasonKleban]

pnpm version: 5.13.6

Code to reproduce the issue:

"preinstall": "npx only-allow pnpm" documented at https://pnpm.js.org/en/only-allow-pnpm is awesome! However it seems impossible to use it in conjunction with package.json5's file extension and its lovely support for comments AND protect against package managers such as yarn which do not recognize .json5.

I thought I'd be clever and create a package.json file that has ONLY the following (in addition to the same script in my package.json5):

{
  "scripts": {
    "preinstall": "npx only-allow pnpm"
  }
}

But unfortunately, pnpm seems to prefer package.json over package.json5. Also, as appropriate, pnpm errors on comments in .json files, so we can't just pretend it's json5/jsonc.

Expected behavior:

pnpm should prefer package.json5 (and yaml) over package.json as the newer and more capable formats.

Actual behavior:

pnpm seems to prefer package.json over package.json5 (and yaml)

[JasonKleban]

I guess the order change would be done in these two places?

pnpm/packages/read-project-manifest/src/index.ts

Lines 60 to 89 in f9befbf

	try {
	const manifestPath = path.join(projectDir, 'package.json')
	const { data, text } = await readJsonFile(manifestPath)
	return {
	fileName: 'package.json',
	manifest: data,
	writeProjectManifest: createManifestWriter({
	...detectFileFormatting(text),
	initialManifest: data,
	manifestPath,
	}),
	}
	} catch (err) {
	if (err.code !== 'ENOENT') throw err
	}
	try {
	const manifestPath = path.join(projectDir, 'package.json5')
	const { data, text } = await readJson5File(manifestPath)
	return {
	fileName: 'package.json5',
	manifest: data,
	writeProjectManifest: createManifestWriter({
	...detectFileFormatting(text),
	initialManifest: data,
	manifestPath,
	}),
	}
	} catch (err) {
	if (err.code !== 'ENOENT') throw err
	}

pnpm/packages/find-packages/src/index.ts

Lines 76 to 81 in f9befbf

	normalizedPatterns.push(
	pattern.replace(/\/?$/, '/package.json')
	)
	normalizedPatterns.push(
	pattern.replace(/\/?$/, '/package.json5')
	)

[JasonKleban]

Ooooh, this seems like a blocker #2008 before this precedence would matter much.

[zkochan]

I guess I don't have objections to change the order but I am also not sure it was a good idea to add json5/yaml support. I received a lot of criticism on Twitter for that. cc @pnpm/collaborators

[juanpicado]

Let's imagine you have package.yaml or package.json5 with comments, on pnpm publish what exactly goes to the registry?

[zkochan]

pnpm converts those files to regular json files before publish

[juanpicado]

I received a lot of criticism on Twitter for that. cc

What was exactly the thoughts other shared?

[zkochan]

https://twitter.com/bitandbang/status/1124993994881155072

[juanpicado]

https://twitter.com/bitandbang/status/1124993994881155072

I see, good point https://nodejs.org/api/packages.html

[nikoladev]

I'd vote for not doing anything that can break workflows. So could this maybe be solved with an opt-in CLI flag?

[aminya]

I recommend JSOX/JSON6:

https://github.com/d3x0r/jsox
https://github.com/d3x0r/JSON6
https://github.com/d3x0r/sack.vfs

[JasonKleban]

Whatever new format are chosen for support, the benefits of human-readability crucially include ordered-member and trivia (whitespace and comments) preservation when the package file is subjected to automated changes (like pnpm add).

I imagine that the simplest way to support this is for the package file updater to merge the newly calculated javascript object data into the original file as a final step - and this will require a parser library with support for merging ASTs with trivia. Are there any next gen notation formats that can provide this? I don't see these features in json5, json6, jsox, jsonc. hjson has -rt/keepWsc for retaining comments and whitespace - but based on how it seems to work, I guess that it cannot preserve object member order?

[bdchauvette]

I am also not sure it was a good idea to add json5/yaml support

IMO it's nice to have in theory, but it's not really useful in practice if you need or want to:

• use dev tools that only look for package.json, e.g. husky@<5, cosmicconfig for non-rc files, eslint-plugin-import
• export metadata from your own package.json, e.g. exports.version = require('./package.json').version (which is admittedly probably not a best practice, but 🤷)
• use "type": "module" to write native ESM modules without having to use .mjs
• use "imports" to get native import aliases

There are probably a lot of people -- maybe even most -- that are fine with the tradeoffs, but I think the presence of a package.json file is such a fundamental assumption in the Node ecosystem that it doesn't really seem worthwhile (IMO) to support non-json configs in the first place.

However, removing support would also be a large breaking change for people that rely on it, so if pnpm were to remove support for non-json configs, it should probably be done gradually, e.g. no change in v5, deprecate in v6, actually remove in v7.

[JasonKleban]

@bdchauvette the world as to keep turning somehow! If pnpm is the near future of package management, then it seems that support for a new format would have to start here, no?

[Chealer]

I guess I don't have objections to change the order but I am also not sure it was a good idea to add json5/yaml support. I received a lot of criticism on Twitter for that.

I must be missing something, but what I see in your link is a short criticism from a single individual (and nothing unexpected).

Thank you for that addition (and thanks Jason for this report)

[vjpr]

I think its worthy of a config setting.

When using package.json5, since many tools rely on package.json, one always needs to be present. It would need to be synced.

We should add a hook called writePackage to pnpmfile.js to allow syncing when running pnpm add|remove|link. See: #5624

[rlidwka]

I agree with this change for the reasons I've explained in #5541 (comment) (which seems to be a duplicate issue).

I am also not sure it was a good idea to add json5/yaml support.

Some would agree with this, others wouldn't. Personally, I've forked npm back in 2014 because of this exact issue (but it didn't catch on at the time), so I'm certainly thankful that I can use json5 here.