Skip to content

Commit a04678c

Browse files
authored
fix: choose correct config directory when require.main does not exist (#1056)
* stop shadowing 'path' import, it's confusing * add no-extension integration test - Verifies that find-up can deal with non-directory startDir - Removes process.chdir() in integration test helper in favor of opts.cwd - Upgrades cross-spawn to v6
1 parent 57a39cb commit a04678c

7 files changed

Lines changed: 67 additions & 22 deletions

File tree

lib/apply-extends.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ const YError = require('./yerror')
66

77
let previouslyVisitedConfigs = []
88

9-
function checkForCircularExtends (path) {
10-
if (previouslyVisitedConfigs.indexOf(path) > -1) {
11-
throw new YError(`Circular extended configurations: '${path}'.`)
9+
function checkForCircularExtends (cfgPath) {
10+
if (previouslyVisitedConfigs.indexOf(cfgPath) > -1) {
11+
throw new YError(`Circular extended configurations: '${cfgPath}'.`)
1212
}
1313
}
1414

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"chalk": "^1.1.3",
3131
"coveralls": "^2.11.11",
3232
"cpr": "^2.0.0",
33-
"cross-spawn": "^5.0.1",
33+
"cross-spawn": "^6.0.4",
3434
"es6-promise": "^4.0.2",
3535
"hashish": "0.0.4",
3636
"mocha": "^3.0.1",

test/fixtures/no-extension

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/usr/bin/env node
2+
3+
'use strict'
4+
5+
var parser = require('../../yargs.js')(process.argv.slice(2))
6+
7+
console.log(parser.argv)

test/fixtures/no-require-main.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#!/usr/bin/env node
2+
3+
'use strict'
4+
5+
// for some unknown reason, a test environment has decided to omit require.main
6+
delete require.main
7+
8+
var parser = require('../../yargs.js')(process.argv.slice(2), undefined, require)
9+
10+
console.log(parser.argv)

test/integration.js

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,17 @@ describe('integration tests', () => {
2929

3030
describe('path returned by "which"', () => {
3131
it('should match the actual path to the script file', (done) => {
32-
which('node', (err, path) => {
32+
which('node', (err, resolvedPath) => {
3333
if (err) return done(err)
34-
testArgs(`${path.replace('Program Files (x86)', 'Progra~2')
34+
testArgs(`${resolvedPath.replace('Program Files (x86)', 'Progra~2')
3535
.replace('Program Files', 'Progra~1')} bin.js`, [], done)
3636
})
3737
})
3838

3939
it('should match the actual path to the script file, with arguments', (done) => {
40-
which('node', (err, path) => {
40+
which('node', (err, resolvedPath) => {
4141
if (err) return done(err)
42-
testArgs(`${path.replace('Program Files (x86)', 'Progra~2')
42+
testArgs(`${resolvedPath.replace('Program Files (x86)', 'Progra~2')
4343
.replace('Program Files', 'Progra~1')} bin.js`, [ 'q', 'r' ], done)
4444
})
4545
})
@@ -169,6 +169,28 @@ describe('integration tests', () => {
169169
return done()
170170
})
171171
})
172+
173+
it('reads parser config settings when somebody obscures require.main', (done) => {
174+
testCmd('./no-require-main.js', [ '--foo.bar' ], (code, stdout) => {
175+
if (code) {
176+
return done(new Error(`cmd exited with code ${code}`))
177+
}
178+
179+
stdout.should.match(/foo\.bar/)
180+
return done()
181+
})
182+
})
183+
184+
it('reads parser config settings when entry file has no extension', (done) => {
185+
testCmd('./no-extension', [ '--foo.bar' ], (code, stdout) => {
186+
if (code) {
187+
return done(new Error(`cmd exited with code ${code}`))
188+
}
189+
190+
stdout.should.match(/foo\.bar/)
191+
return done()
192+
})
193+
})
172194
})
173195

174196
after(() => {
@@ -180,13 +202,11 @@ describe('integration tests', () => {
180202
})
181203

182204
function testCmd (cmd, args, cb) {
183-
const oldDir = process.cwd()
184-
process.chdir(path.join(__dirname, '/fixtures'))
185-
186205
const cmds = cmd.split(' ')
187206

188-
const bin = spawn(cmds[0], cmds.slice(1).concat(args.map(String)))
189-
process.chdir(oldDir)
207+
const bin = spawn(cmds[0], cmds.slice(1).concat(args.map(String)), {
208+
cwd: path.join(__dirname, '/fixtures')
209+
})
190210

191211
let stdout = ''
192212
bin.stdout.setEncoding('utf8')

test/yargs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1187,7 +1187,7 @@ describe('yargs dsl tests', () => {
11871187
describe('config', () => {
11881188
it('allows a parsing function to be provided as a second argument', () => {
11891189
const argv = yargs('--config ./test/fixtures/config.json')
1190-
.config('config', path => JSON.parse(fs.readFileSync(path)))
1190+
.config('config', cfgPath => JSON.parse(fs.readFileSync(cfgPath)))
11911191
.global('config', false)
11921192
.argv
11931193

yargs.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -476,34 +476,42 @@ function Yargs (processArgs, cwd, parentRequire) {
476476
return self
477477
}
478478

479-
self.pkgConf = function pkgConf (key, path) {
480-
argsert('<string> [string]', [key, path], arguments.length)
479+
self.pkgConf = function pkgConf (key, rootPath) {
480+
argsert('<string> [string]', [key, rootPath], arguments.length)
481481
let conf = null
482482
// prefer cwd to require-main-filename in this method
483483
// since we're looking for e.g. "nyc" config in nyc consumer
484484
// rather than "yargs" config in nyc (where nyc is the main filename)
485-
const obj = pkgUp(path || cwd)
485+
const obj = pkgUp(rootPath || cwd)
486486

487487
// If an object exists in the key, add it to options.configObjects
488488
if (obj[key] && typeof obj[key] === 'object') {
489-
conf = applyExtends(obj[key], path || cwd)
489+
conf = applyExtends(obj[key], rootPath || cwd)
490490
options.configObjects = (options.configObjects || []).concat(conf)
491491
}
492492

493493
return self
494494
}
495495

496496
const pkgs = {}
497-
function pkgUp (path) {
498-
const npath = path || '*'
497+
function pkgUp (rootPath) {
498+
const npath = rootPath || '*'
499499
if (pkgs[npath]) return pkgs[npath]
500500
const findUp = require('find-up')
501501

502502
let obj = {}
503503
try {
504+
let startDir = rootPath || require('require-main-filename')(parentRequire || require)
505+
506+
// When called in an environment that lacks require.main.filename, such as a jest test runner,
507+
// startDir is already process.cwd(), and should not be shortened.
508+
// Whether or not it is _actually_ a directory (e.g., extensionless bin) is irrelevant, find-up handles it.
509+
if (!rootPath && path.extname(startDir)) {
510+
startDir = path.dirname(startDir)
511+
}
512+
504513
const pkgJsonPath = findUp.sync('package.json', {
505-
cwd: path || require('path').dirname(require('require-main-filename')(parentRequire || require)),
506-
normalize: false
514+
cwd: startDir
507515
})
508516
obj = JSON.parse(fs.readFileSync(pkgJsonPath))
509517
} catch (noop) {}

0 commit comments

Comments
 (0)