Skip to content

Commit

Permalink
fix: choose correct config directory when require.main does not exist (
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
evocateur committed Jan 31, 2018
1 parent 57a39cb commit a04678c
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 22 deletions.
6 changes: 3 additions & 3 deletions lib/apply-extends.js
Expand Up @@ -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}'.`)
}
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions 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)
10 changes: 10 additions & 0 deletions 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)
38 changes: 29 additions & 9 deletions test/integration.js
Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/yargs.js
Expand Up @@ -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

Expand Down
24 changes: 16 additions & 8 deletions yargs.js
Expand Up @@ -476,34 +476,42 @@ 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)
}

return self
}

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) {}
Expand Down

0 comments on commit a04678c

Please sign in to comment.