Repository: keithamus/sort-package-json
Pull request #277 (2022-11-22 14:14:12)

feat: allow sort `scripts` without `run-s`
Re-submission of #240

CC/ @keithamus @fisker

MichaelDeBoey: @fisker I don't think the Node 12 is about something I've done in this PR, so this one can be merged if everything seems fine to you?
fisker: So, you really think my suggestion should only apply to just one?
keithamus: The node failures we can look into. We may need to drop node 12 support shortly.
MichaelDeBoey: > So, you really think my suggestion should only apply to just one of the patterns?

@fisker Oh I thought it was meant to only have it on one of the regexes.
Since I'm not that good with regexes, can you please create a change request for the others as well?

> The node failures we can look into. We may need to drop node 12 support shortly.

@keithamus The problems with Node v12 CI is because we're always updating to latest `npm` version, which is v9 & is not supporting Node 12 anymore.
If we remove the "always update to latest version" step, Node 12 is perfectly supported & CI will pass
keithamus: If you could please rebase, I've dropped that step in CI.
MichaelDeBoey: @keithamus CI is passing on Node 12 now 👍
fisker: I need more time, but I have to go outside now, will be back in few hours.
fisker: > except the one's mentioned by run-s/-s/--sequential?

Good, but maybe hard to implement.
Personally I  never run sequential tasks with `*`, only parallel tasks.
MichaelDeBoey: In my personal projects I don't do it either, but I've come across it in open source & work projects

For example: when using Remix, it's important that `build:css` is ran before `build:remix` in the Tailwind case: https://remix.run/docs/en/v1/guides/styling#tailwind-css

Don't want to hold back on this PR for that though, so for me this one is approved as well 👍 (can't do it on GitHub, as I created the PR)
fisker: @keithamus Let's merge this?

Patch:
diff --git a/index.js b/index.js
index a176f44..328b6af 100755
--- a/index.js
+++ b/index.js
@@ -218,6 +218,22 @@ const hasDevDependency = (dependency, packageJson) => {
   )
 }
 
+const runSRegExp =
+  /(?<=^|[\s&;<>|(])(?:run-s|npm-run-all .*(?:--sequential|--serial|-s))(?=$|[\s&;<>|)])/
+
+const isSequentialScript = (command) =>
+  command.includes('*') && runSRegExp.test(command)
+
+const hasSequentialScript = (packageJson) => {
+  if (!hasDevDependency('npm-run-all', packageJson)) {
+    return false
+  }
+  const scripts = ['scripts', 'betterScripts'].flatMap((field) =>
+    packageJson[field] ? Object.values(packageJson[field]) : [],
+  )
+  return scripts.some((script) => isSequentialScript(script))
+}
+
 const sortScripts = onObject((scripts, packageJson) => {
   const names = Object.keys(scripts)
   const prefixable = new Set()
@@ -231,10 +247,7 @@ const sortScripts = onObject((scripts, packageJson) => {
     return name
   })
 
-  if (
-    !hasDevDependency('npm-run-all', packageJson) &&
-    !hasDevDependency('npm-run-all2', packageJson)
-  ) {
+  if (!hasSequentialScript(packageJson)) {
     keys.sort()
   }
 


Test patch:
diff --git a/tests/scripts.js b/tests/scripts.js
index 4226901..bc6b210 100644
--- a/tests/scripts.js
+++ b/tests/scripts.js
@@ -1,4 +1,5 @@
 import test from 'ava'
+import sortPackageJson from '../index.js'
 import { macro } from './_helpers.js'
 
 const fixture = {
@@ -35,37 +36,77 @@ const expectAllSorted = {
   watch: 'watch things',
 }
 
-const expectPreAndPostSorted = {
-  pretest: 'xyz',
-  test: 'node test.js',
-  posttest: 'abc',
-  multiply: '2 * 3',
-  prewatch: 'echo "about to watch"',
-  watch: 'watch things',
-  preinstall: 'echo "Installing"',
-  postinstall: 'echo "Installed"',
-  start: 'node server.js',
-  preprettier: 'echo "not pretty"',
-  prettier: 'prettier -l "**/*.js"',
-  postprettier: 'echo "so pretty"',
-  prepare: 'npm run build',
-  'pre-fetch-info': 'foo',
-}
-
 for (const field of ['scripts', 'betterScripts']) {
-  test(`${field} when npm-run-all is not a dev dependency`, macro.sortObject, {
+  test(`${field} when npm-run-all is NOT a dev dependency`, macro.sortObject, {
     value: { [field]: fixture },
     expect: { [field]: expectAllSorted },
   })
-  test(`${field} when npm-run-all is a dev dependency`, macro.sortObject, {
-    value: {
-      [field]: fixture,
-      devDependencies: { 'npm-run-all': '^1.0.0' },
-    },
-    expect: {
-      [field]: expectPreAndPostSorted,
-      devDependencies: { 'npm-run-all': '^1.0.0' },
+
+  test(
+    `${field} when npm-run-all IS a dev dependency, but is NOT used in scripts`,
+    macro.sortObject,
+    {
+      value: {
+        [field]: { z: 'z', a: 'a' },
+        devDependencies: { 'npm-run-all': '^1.0.0' },
+      },
+      expect: {
+        [field]: { a: 'a', z: 'z' },
+        devDependencies: { 'npm-run-all': '^1.0.0' },
+      },
     },
+  )
+}
+
+// `run-s` command
+function sortScriptsWithNpmRunAll(script) {
+  const packageJson = {
+    scripts: { z: 'z', a: 'a', maybeRunS: script },
+    devDependencies: { 'npm-run-all': '^1.0.0' },
+  }
+
+  return Object.keys(sortPackageJson(packageJson).scripts)
+}
+const sortedScripts = ['a', 'maybeRunS', 'z']
+const unsortedScripts = ['z', 'a', 'maybeRunS']
+for (const { script, expected } of [
+  // Should NOT sort
+  { script: 'run-s "lint:*"', expected: unsortedScripts },
+  { script: 'npm-run-all -s "lint:*"', expected: unsortedScripts },
+  { script: 'npm-run-all --sequential "lint:*"', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --sequential', expected: unsortedScripts },
+  { script: 'foo&&npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  { script: 'foo||npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  { script: 'foo|npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  { script: 'foo>npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  { script: 'foo<npm-run-all --serial "lint:*"', expected: unsortedScripts },
+  {
+    script: 'cross-env FOO=1 npm-run-all --serial "lint:*"',
+    expected: unsortedScripts,
+  },
+  { script: 'npm-run-all "lint:*" --serial&&foo', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --serial|foo', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --serial||foo', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --serial>foo', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --serial<foo', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*"&&foo', expected: unsortedScripts },
+  { script: 'npm-run-all "lint:*" --serial;foo', expected: unsortedScripts },
+  { script: '(npm-run-all "lint:*" --serial)|foo', expected: unsortedScripts },
+
+  // Should sort
+  { script: 'run-s lint:a lint:b', expected: sortedScripts },
+  { script: 'not-run-s *', expected: sortedScripts },
+  { script: 'npm-run-all * --serial!', expected: sortedScripts },
+  { script: 'looks like && run-s-but-its-not *', expected: sortedScripts },
+  { script: 'npm-run-all *', expected: sortedScripts },
+  { script: 'npm-run-all --parallel watch:*', expected: sortedScripts },
+
+  // False positive
+  { script: 'rm -rf dist/* && run-s lint:a lint:b', expected: unsortedScripts },
+]) {
+  test(`command: '${script}'`, (t) => {
+    t.deepEqual(sortScriptsWithNpmRunAll(script), expected)
   })
 }
 
@@ -80,7 +121,7 @@ for (const field of ['scripts', 'betterScripts']) {
       devDependencies: { 'npm-run-all2': '^1.0.0' },
     },
     expect: {
-      [field]: expectPreAndPostSorted,
+      [field]: expectAllSorted,
       devDependencies: { 'npm-run-all2': '^1.0.0' },
     },
   })