fix: Resolve value of env variables before invoking cross-spawn (#95)

* fix: Resolve value of env variables before invoking cross-spawn

#90

* Refactored the main parsing loop

BREAKING CHANGE: This is unlikely to break anyone, but now if you assign a variable to a variable (like `FOO=$BAR` with the value `$BAR` being assigned to `hello`, the command will be converted to `FOO=hello` whereas before it was `FOO=$BAR`).

src/index.js

@@ -7,13 +7,17 @@ module.exports = crossEnv
 const envSetterRegex = /(\w+)=('(.+)'|"(.+)"|(.+))/
 
 function crossEnv(args) {
-  const [command, commandArgs, env] = getCommandArgsAndEnvVars(args)
+  const [envSetters, command, commandArgs] = parseCommand(args)
   if (command) {
-    const proc = spawn(command, commandArgs, {
-      stdio: 'inherit',
-      shell: true,
-      env,
-    })
+    const proc = spawn(
+      commandConvert(command),
+      commandArgs.map(commandConvert),
+      {
+        stdio: 'inherit',
+        shell: true,
+        env: getEnvVars(envSetters),
+      },
+    )
     process.on('SIGTERM', () => proc.kill('SIGTERM'))
     process.on('SIGINT', () => proc.kill('SIGINT'))
     process.on('SIGBREAK', () => proc.kill('SIGBREAK'))
@@ -24,30 +28,31 @@ function crossEnv(args) {
   return null
 }
 
-function getCommandArgsAndEnvVars(args) {
-  const envVars = getEnvVars()
-  const commandArgs = args.map(commandConvert)
-  const command = getCommand(commandArgs, envVars)
-  return [command, commandArgs, envVars]
-}
-
-function getCommand(commandArgs, envVars) {
-  while (commandArgs.length) {
-    const shifted = commandArgs.shift()
-    const match = envSetterRegex.exec(shifted)
+function parseCommand(args) {
+  const envSetters = {}
+  let command = null
+  let commandArgs = []
+  for (let i = 0; i < args.length; i++) {
+    const match = envSetterRegex.exec(args[i])
     if (match) {
-      envVars[match[1]] = varValueConvert(match[3] || match[4] || match[5])
+      envSetters[match[1]] = match[3] || match[4] || match[5]
     } else {
-      return shifted
+      // No more env setters, the rest of the line must be the command and args
+      command = args[i]
+      commandArgs = args.slice(i + 1)
+      break
     }
   }
-  return null
+  return [envSetters, command, commandArgs]
 }
 
-function getEnvVars() {
+function getEnvVars(envSetters) {
   const envVars = Object.assign({}, process.env)
   if (process.env.APPDATA) {
     envVars.APPDATA = process.env.APPDATA
   }
+  Object.keys(envSetters).forEach(varName => {
+    envVars[varName] = varValueConvert(envSetters[varName])
+  })
   return envVars
 }

src/variable.js

@@ -1,16 +1,15 @@
 import isWindows from 'is-windows'
 
 /**
- * Converts an environment variable value to be appropriate for the current OS.
- * It will transform UNIX-style list values to Windows-style.
+ * This will transform UNIX-style list values to Windows-style.
  * For example, the value of the $PATH variable "/usr/bin:/usr/local/bin:."
  * will become "/usr/bin;/usr/local/bin;." on Windows.
- * @param {String} originalValue Original value of the env variable
+ * @param {String} varValue Original value of the env variable
  * @returns {String} Converted value
  */
-export default function varValueConvert(originalValue) {
+function replaceListDelimiters(varValue) {
   const targetSeparator = isWindows() ? ';' : ':'
-  return originalValue.replace(/(\\*):/g, (match, backslashes) => {
+  return varValue.replace(/(\\*):/g, (match, backslashes) => {
     if (backslashes.length % 2) {
       // Odd number of backslashes preceding it means it's escaped,
       // remove 1 backslash and return the rest as-is
@@ -19,3 +18,32 @@ export default function varValueConvert(originalValue) {
     return backslashes + targetSeparator
   })
 }
+
+/**
+ * This will attempt to resolve the value of any env variables that are inside
+ * this string. For example, it will transform this:
+ * cross-env FOO=$NODE_ENV echo $FOO
+ * Into this:
+ * FOO=development echo $FOO
+ * (Or whatever value the variable NODE_ENV has)
+ * Note that this function is only called with the right-side portion of the
+ * env var assignment, so in that example, this function would transform
+ * the string "$NODE_ENV" into "development"
+ * @param {String} varValue Original value of the env variable
+ * @returns {String} Converted value
+ */
+function resolveEnvVars(varValue) {
+  const envUnixRegex = /\$(\w+)|\${(\w+)}/g // $my_var or ${my_var}
+  return varValue.replace(envUnixRegex, (_, varName, altVarName) => {
+    return process.env[varName || altVarName] || ''
+  })
+}
+
+/**
+ * Converts an environment variable value to be appropriate for the current OS.
+ * @param {String} originalValue Original value of the env variable
+ * @returns {String} Converted value
+ */
+export default function varValueConvert(originalValue) {
+  return resolveEnvVars(replaceListDelimiters(originalValue))
+}

src/variable.test.js

@@ -2,9 +2,16 @@ import isWindowsMock from 'is-windows'
 import varValueConvert from './variable'
 
 beforeEach(() => {
+  process.env.VAR1 = 'value1'
+  process.env.VAR2 = 'value2'
   isWindowsMock.__mock.reset()
 })
 
+afterEach(() => {
+  delete process.env.VAR1
+  delete process.env.VAR2
+})
+
 test(`doesn't affect simple variable values`, () => {
   isWindowsMock.__mock.returnValue = true
   expect(varValueConvert('foo')).toBe('foo')
@@ -44,3 +51,24 @@ test(`converts multiple separators`, () => {
   isWindowsMock.__mock.returnValue = true
   expect(varValueConvert('foo:bar:baz')).toBe('foo;bar;baz')
 })
+
+test(`resolves an env variable value`, () => {
+  isWindowsMock.__mock.returnValue = true
+  expect(varValueConvert('foo-$VAR1')).toBe('foo-value1')
+})
+
+test(`resolves an env variable value with curly syntax`, () => {
+  isWindowsMock.__mock.returnValue = true
+  // eslint-disable-next-line no-template-curly-in-string
+  expect(varValueConvert('foo-${VAR1}')).toBe('foo-value1')
+})
+
+test(`resolves multiple env variable values`, () => {
+  isWindowsMock.__mock.returnValue = true
+  expect(varValueConvert('foo-$VAR1-$VAR2')).toBe('foo-value1-value2')
+})
+
+test(`resolves an env variable value for non-existant variable`, () => {
+  isWindowsMock.__mock.returnValue = true
+  expect(varValueConvert('foo-$VAR_POTATO')).toBe('foo-')
+})