Enable sorting for npm-run-all

[saiichihashimoto]

I've seen the PRs and reasoning for bailing on script sorting when npm-run-all is involved, but having the behavior drastically change based on the existence of a package is reasonably unexpected and should have been a breaking change, at the least. It makes a lot more sense to have a cli flag for --disable-script-sorting rather than making the behavior impossible if I'm using npm-run-all, so our team is stuck at version 1.48.1 indefinitely. Anyone who's using sort-package-json won't expect this sort of behavior and it's a surprise that can go undetected for months after introducing npm-run-all.

[keithamus]

See #240 for a potentially better heuristic.

Also see #233 (comment) for why I do not wish this project to have any configuration flags.

[saiichihashimoto]

#233 (comment) makes tons of sense to me; my qualm was that a semver major breaking change was made without notification or mitigation path, which was frustrating since it wasn't immediately obvious to us.

#240 is still unpredictable behavior. A specific package will disable script sorting, unless you only use it a specific way. From what I understand, #233 (comment) has the same methodology as prettier; you don't want to bikeshed over opinion where it truly doesn't matter and adding config just adds complexity. I'd argue that this npm-run-all behavior is encoding opinion that does matter. This isn't the difference between tabs and spaces; this is the difference between enabling and disabling large chunks of functionality. We switch between serial and parallel for performance reasons, but now I have to couple that decision with whether I want scripts sorted or not. I can't justify the "either we run scripts in serial or have our scripts sorted" decision.

[keithamus]

same methodology as prettier

Yes sort of, except that prettier takes configuration now. Also in my opinion the lack of configuration (and thus determinism) of this project is fundamental to its benefit...

you don't want to bikeshed over opinion where it truly doesn't matter and adding config just adds complexity

The intent of me making this tool is that I can open a package json and always expect to see (for example) name at the top. If people can override that determinism, then this tool is useless to me. Bikeshedding and complexity are somewhat afterthoughts here.

I'd argue that this npm-run-all behavior is encoding opinion that does matter

I agree. I think that's all this tool does; encode opinions.

We switch between serial and parallel for performance reasons, but now I have to couple that decision with whether I want scripts sorted or not. I can't justify the "either we run scripts in serial or have our scripts sorted" decision.

I mean this in good faith: if you can come up with something better, please do. PRs welcome. This tool does everything I need from it. If it's missing something you want, or does something you think is wrong, let's fix that! As long as it still does what I need (see above comments about intent) then I'm happy for it to do what you need it to also.

[saiichihashimoto]

I mean this in good faith: if you can come up with something better, please do. PRs welcome. This tool does everything I need from it. If it's missing something you want, or does something you think is wrong, let's fix that! As long as it still does what I need (see above comments about intent) then I'm happy for it to do what you need it to also.

This is where we're going to run into an issue. I want to sort scripts... regardless, ie removing any of the npm-run-all specific logic. I'm assuming it's the way it is because that's what you need from it. I'm not sure how to rectify that without config.

[keithamus]

Apologies I wasn't clear. I don't use npm-run-all, this doesn't effect me either way. I've "no skin in this game".

Here's the PR which asked for this behaviour: #206. Here's an issue which is effectively a duplicate of this one: #220. And you've already seen #240 which attempts to appease both camps (the people who depend on scripts to be ordered as they author, and the people who want scripts to be sorted).

Either way this tool works for me and my use - sorting top level keys. I've no strong opinions nor experience to guide me as to what it should do with npm-run-all installed, so I'm simply relying on folks to come forth with solutions that work for them and work for the other people with whom this issue effects.

I'm not sure how to rectify that without config.

Me either 🤷. I do know that configuration will break my use case of determinism, which is why I don't want that.

[saiichihashimoto]

Fair! Sorry for being somewhat aggressive.

It seems like there's only a few options:

1. Keep it the way it is. Con is that its unexpected and arbitrary behavior with no solution for users besides to simply not use run-s. Documentation could help with this, but ultimately isn't a solution.
2. Remove the run-s disabling of sorting. Con is that the sort order of scripts has an effect on serially run scripts.
3. Add config to decide. I agree now that this isn't an option at all, I just want to call it out so it's not a suggestion going forward. The only argument for this is that this was a breaking change but wasn't treated as such.

From what I understand is that the reason we are where we are is because with run-s the order of scripts does make a difference. Since it's running scripts in serial there is an order and sorting the scripts decides the order, which actually changes intention. As far as I can tell, Option 1 and Option 2 are mutually exclusive solutions and, since Option 3 is rightfully off the table, we can't defer choice to the user; this package (ie @keithamus) has to ultimately make a decision unless there's an Option 4 I'm not considering. Let me know if this is the wrong way to look at this.

From what I can tell, here are the reasonably strong arguments for each option:

1. Option 1: Because run-s actually takes into account author order, sort-package-json has a runtime effect, which it shouldn't. Disabling sorting behavior deals with that.
2. Option 2: Controlling the order scripts are run shouldn't be done by package.json sort order. run-s's convenience is that you can name scripts similarly and know that they'll be run in bulk. If sort order of serially run scripts is important, that should be explicit rather than implicit. ie instead of "build": "run-s build:*" being contingent on scripts being defined in a specific order (how would that be clear to anyone?), it should be explicit "build": "run-s codegen:* && run-s build:*" so something as trivial as sorting a config won't break everything. I doubt run-s is the only dependency that could care about script sort order. How many exceptions are we willing to add to sort-package-json? I'd argue that a package named sort-package-json should have the implication that it's taking control of sort order.

Either option's implementation is trivial: for Option 1 we do nothing and for Option 2 we revert the changes that caused it. Also we should have documentation in the README for the decision and, if someone disagrees, referencing the specific version to lock to to get your desired behavior. Ultimately @keithamus needs to decide because there's no true in-between solution.. I'd argue that the proponents for Option 1 are likely louder than for Option 2, considering that proponents for Option 1 are notified of their issue (builds likely breaking) and proponents of Option 2 aren't notified of their issue (I update my dependencies, how would I even know that scripts aren't being sorted anymore). I'm obviously biased because my vote is for Option 2.

[saiichihashimoto]

I made a PR to do this. Ultimately, since it's reverting the change, it comes down to your decision. I can't think of any solution except the three solutions I outlined so I made a PR for the one I prefer.

[keithamus]

Sorry for being somewhat aggressive

I didn't consider anything you said aggressive. I believe we're having a good faith conversation. The written medium is a lossless one though.

How many exceptions are we willing to add to sort-package-json?

Feels like the answer is "as many as necessary". So far we only know of this one.

Documentation could help with this
documentation in the README

Big 👍 here. Perhaps a warning in the console when this behaviour is triggered?

Ultimately @keithamus needs to decide because there's no true in-between solution

To me it seems as though one option leads to broken behaviour and one option leads to broken assumptions. Unless I'm missing something, not sorting scripts does not have any side-effects (other than bemusement and possibly time wasted figuring out why?). Sorting scripts breaks some people's builds though, which means they must discontinue use of the tool for their build to continue working.

Given that information it seems to me Option 1 is the optimal solution. I think refining the heuristic makes it less of a problem for those in the Option 2 camp.

[saiichihashimoto]

In that case, documentation should be added to the README. There's nothing to indicate this will happen.

[saiichihashimoto]

Nothing to do here as the decision was made, closing.