feat: sort all scripts unless npm-run-all is a dependency

[G-Rath]

This restores the default behaviour of sorting scripts alphabetically followed by pre & post unless npm-run-all is a development dependency in the package.json being sorted.

Closes #220

[Maxim-Mazurok]

Doesn't cover npx npm-run-all case, but having npm-run-all as a dev dependency seems like a reasonable requirement in case if I don't want my scripts to be sorted. LGTM 👍

[fisker]

The idea is good, but the implementation is bad, we should not pass packageJson all the way around.

I would check npm-run-all in sortPackageJson function and created a different overFields function.

[G-Rath]

@fisker I'm not sure if I'd say that's a better solution - the advantage I see with passing packageJson around is that it can be used elsewhere for other advanced checks and it's implementation does that in a very clean way by encapsulating the special logic within the function that handles sorting scripts so other functions don't need to know about that.

If you were to make a different overFields function, that'd result in something more complex and special-case-y since it would be devoted purely to the scripts fields, e.g. how would you handle a second independent field with similar behaviour? You'd have a third, maybe even four or more different overFields functions?

That's if I understand your thinking correctly - I've not done a lot of work with this sort of functional currying & piping layout so it's possible I've missed an easy trick :)

[fisker]

that'd result in something more complex and special-case-y since it would be devoted purely to the scripts fields

You are right, I was misleading by the parameter names, what do you think my suggestions bellow?

[fisker]

Suggested change

	const overProperty = (property, over) => (object, packageJson) =>
	hasOwnProperty(object, property)
	? Object.assign(object, { [property]: over(object[property]) })
	? Object.assign(object, { [property]: over(object[property], packageJson) })
	: object
	const overProperty = (property, over) => (object, ...args) =>
	hasOwnProperty(object, property)
	? Object.assign(object, { [property]: over(object[property], ...args) })
	: object

[G-Rath]

what do you think my suggestions?

I'm not too opposed to them, but am interested in what others think as I originally did have the code like that, but the problem I saw was that to me it means you don't have this strong guarantee/implication that the parameter is/should-be the package json that was passed in which is part of the underlying idea: that a field is ensured it'll be passed whatever the value of the field is and the whole package json object.

For me this is the sort of thing where I'd feel more comfortable using a general spread if I was writing TypeScript because I know that'll be able to ensure I'm getting passed the right properties for the field function I'm writing but when writing plain Javascript I prefer having that extra distention.

Additionally, this is all internal API - so shipping it how it is right now shouldn't later prevent switching to using a spread if there becomes a reason to sometimes pass more than one argument or an argument that isn't the whole package json object.

tl;dr: I don't know if there's a lot of value in using spread right now when we're always passing in the same two values, vs the extra clarity of using named parameter 🙂

[Maxim-Mazurok]

I don't think that either approach is too bad. I don't think that dragging json is too bad for perf. But maybe better for ease of understanding. Either way, I haven't contributed to this package, so I'm going to let you guys decide. I'm just excited to see #220 fixed asap :)

[keithamus]

I like the concepts in the code, I also agree with @fisker's assessment and code comments. Let's keep the "lib" stuff generic and then 🚢

[G-Rath]

@keithamus are you ok to just commit the suggestions made by @fisker that you want made, since that should be all that's needed but I'm not at my laptop right now 😅

[github-actions]

🎉 This PR is included in version 1.52.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀

[bcomnes]

Posting here too for visibility: You can also add numerics to your script names to force a specific sort order while retaining the ability to automatically alphanumeric sort. IMO this is preferable than adding tool specific behaviors.