Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

child_process.spawn optional arguments #6068

Closed
alexwhitman opened this issue Aug 17, 2013 · 8 comments
Closed

child_process.spawn optional arguments #6068

alexwhitman opened this issue Aug 17, 2013 · 8 comments

Comments

@alexwhitman
Copy link

The documentation for child_process.spawn() (http://www.nodejs.org/api/child_process.html#child_process_child_process_spawn_command_args_options) suggests that both the args and options arguments are optional. However spawn('ls', { cwd: process.cwd() }) errors with

child_process.js:699
  args = args ? args.slice(0) : [];
                     ^
TypeError: Object #<Object> has no method 'slice'

It can be worked around with spawn('ls', [], { cwd: process.cwd() }).

This is on 0.10.16.

@cdokolas
Copy link

Methinks the correct syntax is spawn('ls', undefined, { cwd: process.cwd() }).

@cjihrig
Copy link

cjihrig commented Sep 10, 2014

This appears to be fixed in at least v0.11.13. Recommend closing. These both work:

Without optional args:

var cp = require('child_process');
var child = cp.spawn('ls', {cwd: '/'});

child.stdout.on('data', function (data) {
  console.log(data.toString());
});

With args:

var cp = require('child_process');
var child = cp.spawn('ls', ['-la'], {cwd: '/'});

child.stdout.on('data', function (data) {
  console.log(data.toString());
});

@trevnorris
Copy link

PR to fix this on v0.10 would be accepted.

@cjihrig
Copy link

cjihrig commented Sep 17, 2014

@trevnorris added a fix for 0.10. I think some cleanup could be done in the latest version. I plan to work on that, if it would be welcomed.

@trevnorris
Copy link

@cjihrig Generally we don't do code cleanup unless it causes the style check to fail, or if the code is being worked on for other reasons. Mainly because it makes life more difficult when trying to trace through a file using git blame.

@cjihrig
Copy link

cjihrig commented Sep 17, 2014

@trevnorris Sorry, I should have been more clear. I didn't mean style fixes. I meant things like this line, which could be simplified down to var env = options.env || process.env;. If you're still not interested, no big deal.

trevnorris pushed a commit that referenced this issue Sep 17, 2014
Currently, a TypeError is incorrectly thrown if the second argument is
an object. This commit allows the args argument to be properly omitted.

Fixes: #6068
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
@trevnorris
Copy link

Fixed by 542ac7f.

@trevnorris
Copy link

@cjihrig That line doesn't even make sense. Yeah, logic check fixes are good.

mscdex pushed a commit to mscdex/node that referenced this issue Dec 25, 2014
Currently, a TypeError is incorrectly thrown if the second argument is
an object. This commit allows the args argument to be properly omitted.

Fixes: nodejs#6068
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
piscisaureus added a commit to nodejs/node that referenced this issue Jan 10, 2015
commit 709558ce0d4027978a742c618d014e5713422263
Author: cjihrig <cjihrig@gmail.com>
Date:   Wed Dec 17 14:58:09 2014 -0500

    test: remove redundant code in test

    A block of asserts were duplicated in
    test/simple/test-child-process-spawn-typeerror.js. This commit
    removes the duplicated asserts.

    Fixes: nodejs/node-v0.x-archive#8454
    Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

commit 9e6d2792ae2eb3ad8553192a1d9c7a7d58808123
Author: Trevor Norris <trev.norris@gmail.com>
Date:   Tue Nov 18 16:42:10 2014 -0800

    lint: fix lint issues

    Forgot to fix these before landing the patch.

    Fixes: e17c5a7

commit 756612776e6bb6b4329a132948f40ad9c28555e2
Author: Sam Roberts <sam@strongloop.com>
Date:   Wed Oct 8 13:30:38 2014 -0700

    test: test all spawn parameter positions

    PR-URL: nodejs/node-v0.x-archive#8454
    Reviewed-by: Trevor Norris <trev.norris@gmail.com>

commit 408812653153cc50f626c7c69b2f8ea4a4be8cec
Author: Sam Roberts <sam@strongloop.com>
Date:   Mon Sep 29 11:26:43 2014 -0700

    child_process: check fork args is an array

    Optional fork args should be type-checked with same behaviour as the
    equivalent argument to spawn.

    PR-URL: nodejs/node-v0.x-archive#8454
    Reviewed-by: Trevor Norris <trev.norris@gmail.com>

commit 11bff9beb2f837a923eb0e515331303f7771301e
Author: Sam Roberts <sam@strongloop.com>
Date:   Thu Sep 25 22:28:33 2014 -0700

    child_process: check execFile args is an array

    execFile and spawn have same API signature with respect to optional arg
    array and optional options object, they should have same behaviour with
    respect to argument validation.

    PR-URL: nodejs/node-v0.x-archive#8454
    Reviewed-by: Trevor Norris <trev.norris@gmail.com>

commit d531ab78c41019f6fdcaf08a1e84ab2b9e439c1b
Author: cjihrig <cjihrig@gmail.com>
Date:   Wed Sep 17 17:26:46 2014 -0400

    child_process: properly support optional args

    Currently, a TypeError is incorrectly thrown if the second argument is
    an object. This commit allows the args argument to be properly omitted.

    Fixes: nodejs/node-v0.x-archive#6068
    Reviewed-by: Trevor Norris <trev.norris@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants