Fix check logic

keithamus · Nov 26, 2022 · 7479e18 · 7479e18
1 parent dd8c480
commit 7479e18
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 74 deletions.
diff --git a/index.js b/index.js
@@ -148,31 +148,25 @@ const hasDevDependency = (dependency, packageJson) => {
   )
 }
 
-const runSValues = [
-  /npm-run-all .* -s(?:$|\s|&|>|<)/,
-  /npm-run-all .* --sequential/,
-  /npm-run-all .* --serial/,
-  /run-s/,
-]
+const runSRegExp = new RegExp(
+  [
+    '(?<=^|[&<>|\\s])',
+    '(?:run-s|npm-run-all .*(?:-s|--sequential|--serial))',
+    '(?=$|[&<>|\\s])',
+  ].join(''),
+)
 
-const hasRunS = (packageJson) => {
+const isSequentialScript = (command) =>
+  command.includes('*') && runSRegExp.test(command)
+
+const hasSequentialScript = (packageJson) => {
   if (!hasDevDependency('npm-run-all', packageJson)) {
     return false
   }
-
-  const scripts = packageJson.scripts
-  const betterScripts = packageJson.betterScripts
-  if (scripts) {
-    return Object.values(scripts).some((script) =>
-      runSValues.some((runSValue) => runSValue.test(script)),
-    )
-  }
-
-  if (betterScripts) {
-    return Object.values(betterScripts).some((script) =>
-      runSValues.some((runSValue) => runSValue.test(script)),
-    )
-  }
+  const scripts = ['scripts', 'betterScripts'].flatMap((field) =>
+    packageJson[field] ? Object.values(packageJson[field]) : [],
+  )
+  return scripts.some((script) => isSequentialScript(script))
 }
 
 const sortScripts = onObject((scripts, packageJson) => {
@@ -188,7 +182,7 @@ const sortScripts = onObject((scripts, packageJson) => {
     return name
   })
 
-  if (!hasRunS(packageJson)) {
+  if (!hasSequentialScript(packageJson)) {
     keys.sort()
   }
 

diff --git 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 = {
@@ -18,23 +19,6 @@ const fixture = {
   'pre-fetch-info': 'foo',
 }
 
-const fixtureWithRunS = {
-  test: 'node test.js',
-  multiply: '2 * 3', // between p(ostinstall) and install
-  watch: 'watch things',
-  prewatch: 'echo "about to watch"',
-  postinstall: 'echo "Installed"',
-  preinstall: 'echo "Installing"',
-  start: 'node server.js',
-  posttest: 'run-s abc def',
-  pretest: 'xyz',
-  postprettier: 'echo "so pretty"',
-  preprettier: 'echo "not pretty"',
-  prettier: 'prettier -l "**/*.js"',
-  prepare: 'npm run build',
-  'pre-fetch-info': 'foo',
-}
-
 const expectAllSorted = {
   preinstall: 'echo "Installing"',
   postinstall: 'echo "Installed"',
@@ -52,57 +36,69 @@ const expectAllSorted = {
   watch: 'watch things',
 }
 
-const expectPreAndPostSorted = {
-  pretest: 'xyz',
-  test: 'node test.js',
-  posttest: 'run-s abc def',
-  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, {
     value: { [field]: fixture },
     expect: { [field]: expectAllSorted },
   })
 
-  for (const type of ['run-s']) {
-    test(
-      `${field} when npm-run-all IS a dev dependency, and IS used in scripts in form of ${type}`,
-      macro.sortObject,
-      {
-        value: {
-          [field]: fixtureWithRunS,
-          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]: fixture,
+        [field]: { z: 'z', a: 'a' },
         devDependencies: { 'npm-run-all': '^1.0.0' },
       },
       expect: {
-        [field]: expectAllSorted,
+        [field]: { a: 'a', z: 'z' },
         devDependencies: { 'npm-run-all': '^1.0.0' },
       },
     },
   )
 }
+
+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']
+// `run-s` command
+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 --serial "lint:*"&&foo', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*"|foo', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*"||foo', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*">foo', expected: unsortedScripts },
+  { script: 'npm-run-all --serial "lint:*"<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 },
+]) {
+  test(`command: '${script}'`, (t) => {
+    t.deepEqual(sortScriptsWithNpmRunAll(script), expected)
+  })
+}