fix: stop sorting scripts

[jonathanmorley]

The fields option will override the values in the builtin fields array,
allowing users to signify that a given key should not be sorted, or sorted
in a particular way.

[fisker]

Why not extend sortOrder to ['name', { key: 'repository', over: myOwnSortFunction }]

[jonathanmorley]

I started going down that route, but if you just specify something like sortOrder: [{ key: 'scripts', over: myOwnSortFunction }] then I wouldn't want that to cause the scripts object to appear at the top of the object.

This way you can influence sort of keys without affecting the ordering of those keys

[fisker]

How about defaultSortOrder.map(key => key === "script" ? { key, over: myOwnSortFunction } : key)

[jonathanmorley]

While that would work, it seems more confusing to me. What is the reasoning behind preferring re-using the sortOrder option rather than introducing a new option?

[jonathanmorley]

what would be needed to move this forward?

[jonathanmorley]

@keithamus ?

[keithamus]

Could you please demonstrate a concrete use case for needing this?

[jonathanmorley]

Sure.

We use npm-run-all to run multiple scripts sequentially, based on their ordering within the scripts block.

An example of this would be

{
  "scripts": {
    "build:ts": "tsc",
    "build:cdk": "cdk synth",
    "build": "run-s build:*"
  }
}

Reordering the scripts block based on lexical ordering would functionally change the build script, in a breaking way.

[keithamus]

@jonathanmorley I would prefer to keep sort-package-json configuration free. So if that is the only use case you're solving, we can:

1. Simply not sort scripts. This is my preferred option.
2. Detect npm-run-all in devdeps and conditionally not sort. I'm willing to entertain this idea with more exploration.

[fisker]

Detect npm-run-all in devdeps and conditionally not sort.

👍

Additional we can also check scripts content has run-s and npm-run-all command. run-p should not effected.

[jonathanmorley]

npm-run-all has a third command (npm-run-all) which also has the ability to run commands sequentially.

Rather than add in checks for npm-run-all or run-s, I opted for the approach 2 in #206 (comment)

[fisker]

What if npm-run-all is installed global. And I mean do both.

[nelson6e65]

Why not just an option to (dis)allow ordering scripts, regardless of the reason, instead of check for a specific tool? What if I use another tool like npm-run-all but is called different? I need to create another patch for each specific tool that requires specific order on scripts?

[jonathanmorley]

that was the original intent. The fields option would allow a consumer to choose to disabling sorting, or choose a custom sort method for a provided field(s). #206 (comment) mentions about wanting to keep sort-package-json config free though, which I can understand, especially as we consume this library through another package, which also has low configuration

[keithamus]

Yeah we can just unilaterally disable sorting of scripts. In my experience these tend to be more taken care of than the rest of the package.json, and the important part of this package is really to sort the main fields so you can predictably head the file to see stuff like name and main

[matzkoh]

I use npm-run-all and prefer to sort the scripts.
For example, it makes sense for prefoo foo postfoo to be sorted in that order.
https://docs.npmjs.com/cli/v6/using-npm/scripts#pre--post-scripts

It would be a shame to stop sorting scripts just because some tools depend on the key order of the objects.
(Rather than "the tools depend on it", the users depend on the behavior that happens to work well?)

It is an object, not an array.

[keithamus]

We could just sort pre and post scripts, as we know them to have consistent behaviour with and without npm-run-all.

[nelson6e65]

We could just sort pre and post scripts, as we know them to have consistent behaviour with and without npm-run-all.

I agree with having pre/post scripts sorted it is useful.

[keithamus]

I'm finding this diff really difficult to read now. Why does it change a bunch of seemingly unrelated lines? Do you have auto formatting on?

[keithamus]

Seems like this is the only LOC that should be in this diff now, no?

[jonathanmorley]

I believe this is prettier moving the .map up a line since it no longer has a .sort to align with

[jonathanmorley]

@keithamus does this sufficiently explain the diff?

[keithamus]

Why has the index.d.ts changed so much? Is it just that the copy in the main branch is old?

[github-actions]

🎉 This PR is included in version 1.49.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀

[G-Rath]

I'm a bit sad that this was merged, especially as a bug: having scripts sorted consistently across projects as a default is one of the things I love(d) about sort-package-json.

Would it be possible to revisit this to see if we could come up with a middleground for everyone?

[joemaller]

I agree with @G-Rath, this change was an unwelcome surprise. We've been using sort-package-json to standardize our package.json files for several years. We use a lot of scripts and keeping those in order was a huge help. After a recent round of updates, I thought something broke when the scripts weren't sorted back into the expected lexical+pre/post order.

I see sort-package-json as a linting tool, and much as I've given formatting over to tools like Prettier, I'm fine with whatever opinionated choices the tool's maintainers decide to make. But removing script sorting is a major loss of functionality which I'll be looking to replace. For now, running npx sort-package-json@1.48 was a sigh of relief.

[benquarmby]

I agree with @G-Rath, this change was an unwelcome surprise.

I'd go so far as to call it a breaking change. We will use ~1.48.0 as an escape hatch for now.

Edit: This edge case really wasn't very hard to solve and costs very little:

{
  "scripts": {
    "build": "run-s build:ts build:cdk",
    "build:cdk": "cdk synth",
    "build:ts": "tsc"
  }
}

[DUBANGARCIA]

I agree with @G-Rath, this change was an unwelcome surprise. We've been using sort-package-json to standardize our package.json files for several years. We use a lot of scripts and keeping those in order was a huge help. After a recent round of updates, I thought something broke when the scripts weren't sorted back into the expected lexical+pre/post order.

I see sort-package-json as a linting tool, and much as I've given formatting over to tools like Prettier, I'm fine with whatever opinionated choices the tool's maintainers decide to make. But removing script sorting is a major loss of functionality which I'll be looking to replace. For now, running npx sort-package-json@1.48 was a sigh of relief.

Totally agree, the ordering of scripts was very important for my projects to avoid unnecessary merges with git and improve the readability of the package.json, please reconsider this change.

[Maxim-Mazurok]

I just want to let you know that it also was a surprise that our scripts are not being sorted and I just noticed this... Guess it is me to blame because I didn't pin this package version and was just using the latest.

But yeah, I agree with the guys above that this was a pretty important feature for me, prettier/linter for pacakge.json, in a way. And I'm sad to see this functionality gone. Will pin to 1.48 for now.

[keithamus]

This edge case really wasn't very hard to solve and costs very little

Please do submit a PR if you think this is solvable in a deterministic and non-configurable way that addresses the concerns of folks like @jonathanmorley.

[benquarmby]

@keithamus : The deterministic way was covered in the code example. To be clear, here it is again:

- "build": "run-s build:*",
+ "build": "run-s build:ts build:cdk",

Being explicit about the order and avoiding the magical * is as deterministic as it gets. I say this as someone who makes heavy use of npm-run-all.

I'll be blunt: sort-package-json should never have been watered down to cater for dynamic usage of npm-run-all. It was a breaking change for my projects.

[keithamus]

@G-Rath has submitted a PR which sorts scripts unless npm-run-all is installed. It is released under 1.52.0

[nelson6e65]

Hmm... What if I have installed and still want to sort them? Examples above shows that.

[Maxim-Mazurok]

Hmm... What if I have installed and still want to sort them? Examples above shows that.

I guess you have two choices:

1. Install it as a regular (non-dev) dependency. Should be fine for non-library projects. (in Script not sorting in 1.49.0 #220 they check that npm-run-all is a dev dependency)
2. Uninstall it and use through npx

[keithamus]

If you would like to improve the heuristic - if you think you could find a deterministic way to sort scripts without effecting existing implicit npm-run-all script orders, then PRs are welcome! For example we could check the contents of scripts to determine if they're using run-s with a wildcard.

[fisker]

Don't forget npm-run-all foo:* -s, npm-run-all --sequential foo*, npm-run-all --serial foo*, and other cases.

[kachkaev]

Not sure if this is relevant, but npm-run-all has been unmaintained since October 2019. I tried bumping dependencies to get rid of known vulnerabilities, but my PR did not get any attention so far.

We might need to implement a rule that is flexible, i.e. not hard-wired to npm-run-all.

[saiichihashimoto]

Went through a "bisect" of sort-package-json versions until I ended up here to determine this was not a bug, but a conscious decision. This is definitely a breaking change and magic sorting (or lack of sorting?) because a dependency is included is unpredictable behavior. I use npm-run-all extensively and the lack of sorting is painful.

Users of sort-package-json should assume that the json will be resorted (excluding arrays); relying on ordering of object keys is not a bug within sort-package-json, but relying on an unreliability. sort-package-json or npm-run-all should not consider this an issue, considering npm-run-all gives escape hatches for this. If there's a manual order you need scripts to be run, then manually run them in that order. run-s * and run-p * should be agnostic to the order they're run.

Will be pinning to 1.48.1. Regardless of whether the maintainer agrees with the suggested changes, breaking changes like this should definitely be a major version bump, since users reasonably assume a minor version bump wouldn't disable default behavior arbitrarily.