fix: Normalize command only and not command args(windows) (#134)

* fix: Normalize command only and not command args(windows)

Command arguments can have URLs which should not be normalized. Add optional parameter
`commandConvert`(default `false`) and normalize only the command, not the command arguments

* test: Add tests for cmd normalization on windows

index.test.js
  - should normalize command on windows
  - should not normalize command arguments on windows
command.test.js - normalizes command on windows

src/command.js

@@ -6,12 +6,19 @@ export default commandConvert
 /**
  * Converts an environment variable usage to be appropriate for the current OS
  * @param {String} command Command to convert
+ * @param {boolean} normalize If the command should be normalized using `path`
+ * after converting
  * @returns {String} Converted command
  */
-function commandConvert(command) {
+function commandConvert(command, normalize = false) {
   if (!isWindows()) {
     return command
   }
   const envUnixRegex = /\$(\w+)|\${(\w+)}/g // $my_var or ${my_var}
-  return path.normalize(command.replace(envUnixRegex, '%$1$2%'))
+  const convertedCmd = command.replace(envUnixRegex, '%$1$2%')
+  // Normalization is required for commands with relative paths
+  // For example, `./cmd.bat`. See kentcdodds/cross-env#127
+  // However, it should not be done for command arguments.
+  // See https://github.com/kentcdodds/cross-env/pull/130#issuecomment-319887970
+  return normalize === true ? path.normalize(convertedCmd) : convertedCmd
 }

src/command.test.js

@@ -1,4 +1,3 @@
-import path from 'path'
 import isWindowsMock from 'is-windows'
 import commandConvert from './command'
 
@@ -35,7 +34,7 @@ test(`is stateless`, () => {
 test(`converts embedded unix-style env variables usage for windows`, () => {
   isWindowsMock.__mock.returnValue = true
   expect(commandConvert('$test1/$test2/$test3')).toBe(
-    path.normalize('%test1%/%test2%/%test3%'),
+    '%test1%/%test2%/%test3%',
   )
 })
 
@@ -53,3 +52,10 @@ test(`converts braced unix-style env variable usage for windows`, () => {
   // eslint-disable-next-line no-template-curly-in-string
   expect(commandConvert('${test}')).toBe('%test%')
 })
+
+test(`normalizes command on windows`, () => {
+  isWindowsMock.__mock.returnValue = true
+  // index.js calls `commandConvert` with `normalize` param
+  // as `true` for command only
+  expect(commandConvert('./cmd.bat', true)).toBe('cmd.bat')
+})

src/index.js

@@ -10,7 +10,9 @@ function crossEnv(args, options = {}) {
   const [envSetters, command, commandArgs] = parseCommand(args)
   if (command) {
     const proc = spawn(
-      commandConvert(command),
+      // run `path.normalize` for command(on windows)
+      commandConvert(command, true),
+      // by default normalize is `false`, so not run for cmd args
       commandArgs.map(commandConvert),
       {
         stdio: 'inherit',

src/index.test.js

@@ -1,4 +1,5 @@
 import crossSpawnMock from 'cross-spawn'
+import isWindowsMock from 'is-windows'
 
 const crossEnv = require('.')
 
@@ -78,6 +79,30 @@ it(`should do nothing given no command`, () => {
   expect(crossSpawnMock.spawn).toHaveBeenCalledTimes(0)
 })
 
+it(`should normalize command on windows`, () => {
+  isWindowsMock.__mock.returnValue = true
+  crossEnv(['./cmd.bat'])
+  expect(crossSpawnMock.spawn).toHaveBeenCalledWith('cmd.bat', [], {
+    stdio: 'inherit',
+    env: Object.assign({}, process.env),
+  })
+  isWindowsMock.__mock.reset()
+})
+
+it(`should not normalize command arguments on windows`, () => {
+  isWindowsMock.__mock.returnValue = true
+  crossEnv(['echo', 'http://example.com'])
+  expect(crossSpawnMock.spawn).toHaveBeenCalledWith(
+    'echo',
+    ['http://example.com'],
+    {
+      stdio: 'inherit',
+      env: Object.assign({}, process.env),
+    },
+  )
+  isWindowsMock.__mock.reset()
+})
+
 it(`should propagate kill signals`, () => {
   testEnvSetting({FOO_ENV: 'foo=bar'}, 'FOO_ENV="foo=bar"')