🐛 Bug: `sort-collections`: `preinstall` and `postinstall` hooks aren't sorted as co-located to `install`

[rakleed]

Bug Report Checklist

• [x] I have tried restarting my IDE and the issue persists.
• [x] I have pulled the latest main branch of the repository.
• [x] I have searched for related issues and found none that matched my issue.

Expected

{
  "scripts": {
    "build": "vite build",
    "dev": "vite",
    "preinstall": "echo",
    "postinstall": "patch-package && echo && nolyfill install",
    "lint": "eslint .",
    "preview": "vite preview"
  },
}

Actual

{
  "scripts": {
    "build": "vite build",
    "dev": "vite",
    "lint": "eslint .",
    "postinstall": "patch-package && echo && nolyfill install",
    "preinstall": "echo",
    "preview": "vite preview"
  },
}

Additional Info

Although the prettier-plugin-packagejson plugin sorts as I expect, so they are currently incompatible for this case.

[JoshuaKGoldberg]

Looks like a case related to https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/issues/499 -> https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/pull/632. cc @sasial-dev from #632 - what do you think?

[sasial-dev]

That's interesting! I definitely think it could be a bug in my implimentation.

[sasial-dev]

After looking at it again - is it working as expected @rakleed? I notice that there's no install script in that package.json for it to colocate with? And so that's why it's down with the scripts with the first letter of p.

[sasial-dev]

And also - what happens when you add an install script to your package.json?

[rakleed]

I notice that there's no install script in that package.json for it to colocate with? And so that's why it's down with the scripts with the first letter of p.

But prettier-plugin-packagejson can handle such a case.

And also - what happens when you add an install script to your package.json?

In that case, it sorts as expected. But I have never seen anyone rewrite the install script.

[sasial-dev]

I get what you mean now! I totally didn't forget the install script was built in. The question we have now is do we consider it intended behaviour or do we create a list of builtin scripts that are considered colocated?

[michaelfaith]

I think the latter. At the very least, as a user, I would expect preinstall to come before postinstall. Now, whether they're sorted with the "i"s, I don't really have an opinion. So, if we're going to need to make a change anyway, perhaps aligning with prettier-plugin-packagejson should be a goal?

[rakleed]

perhaps aligning with prettier-plugin-packagejson should be a goal?

It is based on sort-package-json, but it seems that they updated defaultNpmScripts last time 6 years ago, so I am not sure if it's complete:

https://github.com/keithamus/sort-package-json/blame/819cd97d787e81c7cb12a618bd07dc100c4bc9f8/index.js#L199-L212

[rakleed]

@JoshuaKGoldberg what do you think?

[JoshuaKGoldberg]

perhaps aligning with prettier-plugin-packagejson should be a goal?

It is based on sort-package-json, but it seems that they updated defaultNpmScripts last time 6 years ago, so I am not sure if it's complete:

https://github.com/keithamus/sort-package-json/blame/main/index.js#L209-L222

They do have 'install' in that list though, so we should be able to rely on it as a built-in. I think any missing built-ins from the last 6 years would be a separate issue from this one.

I'm 👍 on a feature request to align with prettier-plugin-packagejson. Sounds like that's the consensus here?

[rakleed]

@JoshuaKGoldberg @michaelfaith should the status of the issue be changed to https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/labels/status%3A%20accepting%20prs?

[michaelfaith]

Thanks for the nudge. Yeah, i think we've determined there's work to do here. 👍

[JoshuaKGoldberg]

@all-contributors please add @rakleed for bug.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action. Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed. ...and of course, thank you for contributing! 💙

[allcontributors[bot]]

@JoshuaKGoldberg

I've put up a pull request to add @rakleed! :tada:

[github-actions[bot]]

:tada: This is included in version v0.52.1 :tada:

The release is available on:

• GitHub releases
• npm package (@latest dist-tag)

Cheers! 📦🚀