Repository: yargs/yargs
Pull request #1056 (2018-01-24 00:25:34)

fix: choose correct config directory when require.main does not exist
So, it turns out Jest overrides the module-local `require` and neglects to assign `require.main.filename` (https://github.com/facebook/jest/issues/2150). Maybe I'll even get around to fixing it [here](https://github.com/facebook/jest/blob/be66ccc26ea80e825f2fb4f0f21433832a2c4d97/packages/jest-runtime/src/index.js#L675-L689), but in the meantime, we can work around it by avoiding the "extra" `path.dirname()` when it isn't neccessary (because `require-main-filename` defaults to `process.cwd()`).

Also, I changed a bunch of function parameters to avoid shadowing `path`, it was extremely confusing (and ridiculous to have an inner `require('path').dirname` when `const path = require('path')` is already present at the top of the file).

evocateur: This fixes the (untested) fix from #1036, which exposed jest's nefarious activity.

Patch:
diff --git a/lib/apply-extends.js b/lib/apply-extends.js
index 3005848..530b022 100644
--- a/lib/apply-extends.js
+++ b/lib/apply-extends.js
@@ -6,9 +6,9 @@ const YError = require('./yerror')
 
 let previouslyVisitedConfigs = []
 
-function checkForCircularExtends (path) {
-  if (previouslyVisitedConfigs.indexOf(path) > -1) {
-    throw new YError(`Circular extended configurations: '${path}'.`)
+function checkForCircularExtends (cfgPath) {
+  if (previouslyVisitedConfigs.indexOf(cfgPath) > -1) {
+    throw new YError(`Circular extended configurations: '${cfgPath}'.`)
   }
 }
 
diff --git a/package.json b/package.json
index adc1cba..28f2167 100644
--- a/package.json
+++ b/package.json
@@ -30,7 +30,7 @@
     "chalk": "^1.1.3",
     "coveralls": "^2.11.11",
     "cpr": "^2.0.0",
-    "cross-spawn": "^5.0.1",
+    "cross-spawn": "^6.0.4",
     "es6-promise": "^4.0.2",
     "hashish": "0.0.4",
     "mocha": "^3.0.1",
diff --git a/yargs.js b/yargs.js
index 9b50b8b..38f1b2e 100644
--- a/yargs.js
+++ b/yargs.js
@@ -476,17 +476,17 @@ function Yargs (processArgs, cwd, parentRequire) {
     return self
   }
 
-  self.pkgConf = function pkgConf (key, path) {
-    argsert('<string> [string]', [key, path], arguments.length)
+  self.pkgConf = function pkgConf (key, rootPath) {
+    argsert('<string> [string]', [key, rootPath], arguments.length)
     let conf = null
     // prefer cwd to require-main-filename in this method
     // since we're looking for e.g. "nyc" config in nyc consumer
     // rather than "yargs" config in nyc (where nyc is the main filename)
-    const obj = pkgUp(path || cwd)
+    const obj = pkgUp(rootPath || cwd)
 
     // If an object exists in the key, add it to options.configObjects
     if (obj[key] && typeof obj[key] === 'object') {
-      conf = applyExtends(obj[key], path || cwd)
+      conf = applyExtends(obj[key], rootPath || cwd)
       options.configObjects = (options.configObjects || []).concat(conf)
     }
 
@@ -494,16 +494,24 @@ function Yargs (processArgs, cwd, parentRequire) {
   }
 
   const pkgs = {}
-  function pkgUp (path) {
-    const npath = path || '*'
+  function pkgUp (rootPath) {
+    const npath = rootPath || '*'
     if (pkgs[npath]) return pkgs[npath]
     const findUp = require('find-up')
 
     let obj = {}
     try {
+      let startDir = rootPath || require('require-main-filename')(parentRequire || require)
+
+      // When called in an environment that lacks require.main.filename, such as a jest test runner,
+      // startDir is already process.cwd(), and should not be shortened.
+      // Whether or not it is _actually_ a directory (e.g., extensionless bin) is irrelevant, find-up handles it.
+      if (!rootPath && path.extname(startDir)) {
+        startDir = path.dirname(startDir)
+      }
+
       const pkgJsonPath = findUp.sync('package.json', {
-        cwd: path || require('path').dirname(require('require-main-filename')(parentRequire || require)),
-        normalize: false
+        cwd: startDir
       })
       obj = JSON.parse(fs.readFileSync(pkgJsonPath))
     } catch (noop) {}


Test patch:
diff --git a/test/fixtures/no-extension b/test/fixtures/no-extension
new file mode 100755
index 0000000..85c5620
--- /dev/null
+++ b/test/fixtures/no-extension
@@ -0,0 +1,7 @@
+#!/usr/bin/env node
+
+'use strict'
+
+var parser = require('../../yargs.js')(process.argv.slice(2))
+
+console.log(parser.argv)
diff --git a/test/fixtures/no-require-main.js b/test/fixtures/no-require-main.js
new file mode 100755
index 0000000..3e46c8b
--- /dev/null
+++ b/test/fixtures/no-require-main.js
@@ -0,0 +1,10 @@
+#!/usr/bin/env node
+
+'use strict'
+
+// for some unknown reason, a test environment has decided to omit require.main
+delete require.main
+
+var parser = require('../../yargs.js')(process.argv.slice(2), undefined, require)
+
+console.log(parser.argv)
diff --git a/test/integration.js b/test/integration.js
index a47812d..232f493 100644
--- a/test/integration.js
+++ b/test/integration.js
@@ -29,17 +29,17 @@ describe('integration tests', () => {
 
   describe('path returned by "which"', () => {
     it('should match the actual path to the script file', (done) => {
-      which('node', (err, path) => {
+      which('node', (err, resolvedPath) => {
         if (err) return done(err)
-        testArgs(`${path.replace('Program Files (x86)', 'Progra~2')
+        testArgs(`${resolvedPath.replace('Program Files (x86)', 'Progra~2')
                      .replace('Program Files', 'Progra~1')} bin.js`, [], done)
       })
     })
 
     it('should match the actual path to the script file, with arguments', (done) => {
-      which('node', (err, path) => {
+      which('node', (err, resolvedPath) => {
         if (err) return done(err)
-        testArgs(`${path.replace('Program Files (x86)', 'Progra~2')
+        testArgs(`${resolvedPath.replace('Program Files (x86)', 'Progra~2')
                      .replace('Program Files', 'Progra~1')} bin.js`, [ 'q', 'r' ], done)
       })
     })
@@ -169,6 +169,28 @@ describe('integration tests', () => {
             return done()
           })
         })
+
+        it('reads parser config settings when somebody obscures require.main', (done) => {
+          testCmd('./no-require-main.js', [ '--foo.bar' ], (code, stdout) => {
+            if (code) {
+              return done(new Error(`cmd exited with code ${code}`))
+            }
+
+            stdout.should.match(/foo\.bar/)
+            return done()
+          })
+        })
+
+        it('reads parser config settings when entry file has no extension', (done) => {
+          testCmd('./no-extension', [ '--foo.bar' ], (code, stdout) => {
+            if (code) {
+              return done(new Error(`cmd exited with code ${code}`))
+            }
+
+            stdout.should.match(/foo\.bar/)
+            return done()
+          })
+        })
       })
 
       after(() => {
@@ -180,13 +202,11 @@ describe('integration tests', () => {
 })
 
 function testCmd (cmd, args, cb) {
-  const oldDir = process.cwd()
-  process.chdir(path.join(__dirname, '/fixtures'))
-
   const cmds = cmd.split(' ')
 
-  const bin = spawn(cmds[0], cmds.slice(1).concat(args.map(String)))
-  process.chdir(oldDir)
+  const bin = spawn(cmds[0], cmds.slice(1).concat(args.map(String)), {
+    cwd: path.join(__dirname, '/fixtures')
+  })
 
   let stdout = ''
   bin.stdout.setEncoding('utf8')
diff --git a/test/yargs.js b/test/yargs.js
index 1945228..94f4018 100644
--- a/test/yargs.js
+++ b/test/yargs.js
@@ -1187,7 +1187,7 @@ describe('yargs dsl tests', () => {
   describe('config', () => {
     it('allows a parsing function to be provided as a second argument', () => {
       const argv = yargs('--config ./test/fixtures/config.json')
-        .config('config', path => JSON.parse(fs.readFileSync(path)))
+        .config('config', cfgPath => JSON.parse(fs.readFileSync(cfgPath)))
         .global('config', false)
         .argv