Skip to content

Commit

Permalink
fix: package.main with -- arguments (#1773)
Browse files Browse the repository at this point in the history
* fix: package.main with -- arguments

Fixes #1758

The combination of using a package.main (which sets the script
position to index zero) and using the -- stop slurp meant that
the arguments had the script appended to the end instead of
prepended to the start. The net result meant that when the script
was forked, it would drop the first user arg.

See diff for details of the fix - a simple check against null.

* fix: protect against missing opts
  • Loading branch information
remy committed Oct 4, 2020
1 parent 273d774 commit 2967726
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 2 deletions.
4 changes: 3 additions & 1 deletion lib/config/load.js
Expand Up @@ -74,7 +74,9 @@ function load(settings, options, config, callback) {
}
// if the script is found as a result of not being on the command
// line, then we move any of the pre double-dash args in execArgs
const n = options.scriptPosition || options.args.length;
const n = options.scriptPosition === null ?
options.args.length : options.scriptPosition;

options.execArgs = (options.execArgs || [])
.concat(options.args.splice(0, n));
options.scriptPosition = null;
Expand Down
2 changes: 2 additions & 0 deletions lib/monitor/run.js
Expand Up @@ -97,6 +97,8 @@ function run(options) {
utils.version.major > 4 // only fork if node version > 4

if (shouldFork) {
// this assumes the first argument is the script and slices it out, since
// we're forking
var forkArgs = cmd.args.slice(1);
var env = utils.merge(options.execOptions.env, process.env);
stdio.push('ipc');
Expand Down
2 changes: 1 addition & 1 deletion lib/monitor/watch.js
Expand Up @@ -177,7 +177,7 @@ function filterAndRestart(files) {

// if there's no matches, then test to see if the changed file is the
// running script, if so, let's allow a restart
if (config.options.execOptions.script) {
if (config.options.execOptions && config.options.execOptions.script) {
const script = path.resolve(config.options.execOptions.script);
if (matched.result.length === 0 && script) {
const length = script.length;
Expand Down
23 changes: 23 additions & 0 deletions test/config/load.test.js
Expand Up @@ -298,4 +298,27 @@ describe('config load', function () {
done();
})
});

it('should support pkg.main and keep user args on args', done => {
process.chdir(path.resolve(pwd, 'test/fixtures/packages/main-and-start'));
const settings = { scriptPosition: 0, script: null, args: [ 'first', 'second' ] };
const options = { ignore: [], watch: [], monitor: [] };
const config = {
run: false,
system: { cwd: '/Users/remy/dev/nodemon/issues/1758' },
required: false,
dirs: [],
timeout: 1000,
options: { ignore: [], watch: [], monitor: [] },
lastStarted: 0,
loaded: []
}

load(settings, options, config, res => {
assert.deepEqual(res.execOptions.args, ['first', 'second']);
done();
})
});


});
Empty file.
6 changes: 6 additions & 0 deletions test/fixtures/packages/main-and-start/package.json
@@ -0,0 +1,6 @@
{
"main": "./index.js",
"scripts": {
"start": "node index.js"
}
}

0 comments on commit 2967726

Please sign in to comment.