added process.exit -> window.close

[vicapow]

added support for process.exit()

[thlorenz]

I'd argue that process.exit should only ever be called in ./bin/cli script. So it follows that that code would never make it into the browser in the first place.

At the most it should be something like: process.exit = function (code) { console.log('process exited with', code); }.

[vicapow]

well, I have code that I'd like to run in a browser that depends on it being a thing. My use case is basically the same as this persons: https://github.com/substack/node-browserify/issues/242

[juliangruber]

+1

If a lib does this it either should only be used as a cli script or sucks, so it should be safe enough.

[defunctzombie]

absolutely -1 on the console.log. Will not merge with a message being printed to console for no reason. Otherwise, I think it is reasonable to include this as it is minimal code and maybe someone might want to exit "close" the window as noted above.

[thlorenz]

So console.log and closing the window doesn't make much sense since you couldn't see the log. However even just closing the window is problematic. You'll be having lots of fun tracking down who is closing your window, since 'pooof' and it's gone and you don't know why.

As mentioned before process.exit shouldn't make it into the browser in the first place, but at least console logging it allows you to easily get the info that the process was supposed to exit and track down the culprit. Then you should make a PR on that module to remove the process.exit invocation.

[defunctzombie]

I would favor a throw in that case over a console.log. I don't like things that feel they need to output to console without me explicitly asking them. A throw indicates unsupported behavior which is what this method currently is.

[defunctzombie]

@vicapow can you outline what code you have that requires this to be a thing? Is this code you have control over or is it misbehaving?

[vicapow]

(sorry for the double post. last time i tried to send this message i was logged in as a different user.)

@defunctzombie I was using mocha in combination with a custom reporter that I wanted to have bail out early without proceeding remaining tests when the current test fails. this reporter was originally written for node but when I tried to use it in the browser with mocha, I got an error that process.exit was undefined. I have control over this code so as a fix I just added:

if (typeof window === 'undefined') process.exit()
else window.close()

not a big deal to fix but I thought it would be something other people might run into as well. I want the tests to close the browser so that phantomJS can notice the window closed and finally produce the report without other reports having been run.

 // window.close was called to trigger the onClosing phantomJS event
page.onClosing = function(page){
  exit();
}

[defunctzombie]

Interesting use case. The thing that makes me weary of it is some random process.exit() call going undetected. Maybe this is an unwarranted concern. I would like to get some more feedback from other browserify users so will give it a few more days of discussion. I know this might seem trivial but I at least want the change (if any happens) to have some verbal visibility so we can deal with any issues down the road.

/cc @substack @raynos

[defunctzombie]

I am closing this as I don't think we should be adding this behavior into the process code. The process shim here exists to provide the most minimal shimming to make code work.