From 2ddcff30c64c4ab8329214d59fb261f1213dc494 Mon Sep 17 00:00:00 2001
From: Raees Iqbal <raees.bhatti@gmail.com>
Date: Thu, 18 Jun 2020 22:15:08 -0700
Subject: [PATCH] Fix checks for "command" and "dir" combination (#936)

* Fix checks for "command" and "dir" combination

* Add tests for new dir flag behaviour

* Formatting
---
 src/utils/detect-server.js      |  4 ++--
 src/utils/detect-server.test.js | 24 +++++++++++++++++++-----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/utils/detect-server.js b/src/utils/detect-server.js
index fa91fe19ebb..606cbc66433 100644
--- a/src/utils/detect-server.js
+++ b/src/utils/detect-server.js
@@ -15,7 +15,7 @@ module.exports.serverSettings = async (devConfig, flags, projectDir, log) => {
   if (flags.dir) {
     settings = await getStaticServerSettings(settings, flags, projectDir, log)
     ;['command', 'targetPort'].forEach(p => {
-      if (devConfig.hasOwnProperty(p)) {
+      if (flags[p]) {
         throw new Error(
           `"${p}" option cannot be used in conjunction with "dir" flag which is used to run a static server`
         )
@@ -96,7 +96,7 @@ module.exports.serverSettings = async (devConfig, flags, projectDir, log) => {
   if (settings.command === 'npm' && !['start', 'run'].includes(settings.args[0])) {
     settings.args.unshift('run')
   }
-  if (devConfig.command) {
+  if (!settings.noCmd && devConfig.command) {
     settings.command = assignLoudly(devConfig.command.split(/\s/)[0], settings.command || null, tellUser('command')) // if settings.command is empty, its bc no settings matched
     settings.args = assignLoudly(devConfig.command.split(/\s/).slice(1), [], tellUser('command')) // if settings.command is empty, its bc no settings matched
   }
diff --git a/src/utils/detect-server.test.js b/src/utils/detect-server.test.js
index de7ec67eb86..fae180e3a3b 100644
--- a/src/utils/detect-server.test.js
+++ b/src/utils/detect-server.test.js
@@ -110,17 +110,31 @@ test('serverSettings: "dir" flag', async t => {
   t.is(settings.noCmd, true)
 })
 
-test('serverSettings: "dir" flag with "targetPort"', async t => {
-  const devConfig = { framework: '#auto', targetPort: 1234, functions: path.join(sitePath, 'functions') }
+test('serverSettings: "dir" flag and "command" as config param', async t => {
+  const devConfig = {
+    framework: '#auto',
+    command: 'npm start',
+    publish: path.join(sitePath, 'build'),
+    functions: path.join(sitePath, 'functions'),
+  }
   const flags = { dir: sitePath }
+  const settings = await serverSettings(devConfig, flags, sitePath, () => {})
+  t.is(settings.command, undefined)
+  t.is(settings.noCmd, true)
+  t.is(settings.dist, flags.dir)
+})
+
+test('serverSettings: "dir" and "targetPort" flags', async t => {
+  const devConfig = { framework: '#auto', functions: path.join(sitePath, 'functions') }
+  const flags = { dir: sitePath, targetPort: 1234 }
   await t.throwsAsync(async () => {
     await serverSettings(devConfig, flags, sitePath, () => {})
   }, /"targetPort" option cannot be used in conjunction with "dir" flag/)
 })
 
-test('serverSettings: "dir" flag with "command"', async t => {
-  const devConfig = { framework: '#auto', command: 'ding', functions: path.join(sitePath, 'functions') }
-  const flags = { dir: sitePath }
+test('serverSettings: "dir" and "command" flags', async t => {
+  const devConfig = { framework: '#auto', functions: path.join(sitePath, 'functions') }
+  const flags = { dir: sitePath, command: 'ding' }
   await t.throwsAsync(async () => {
     await serverSettings(devConfig, flags, sitePath, () => {})
   }, /"command" option cannot be used in conjunction with "dir" flag/)