🐛 fix: Fixes non-global Phantom use

[dflynn15]

This should fix the Windows specific path issues that @MNR1 was seeing after #66 was merged in. Also, this is the alternative to #70 that does not require us to explicitly download and install phantomjs during npm install but opts to allow user configuration. Explanation here.

@gwynjudd and @MNR1 would you mind double checking to ensure this does not break either of your environments? Thanks!

[mnmercer]

Unfortunately, I won't be able to test this in the same environment I was using until Monday. However, I have made a small test project on my personal Windows 8.1. When I attempt to require the plugin in my gulpfile.js, I get an error saying that getExecutablePath is not defined when it is called on line 22.

[mnmercer]

Confirmed same error on a Ubuntu 16.04 machine.

[mnmercer]

Looked into the issue and two changes have needed to be changed before the plugin could run. The name of the function on line 33 should be getExecuteablePath and line 34 should be

return process.platform === 'win32' ?

With these changes, tests were run successfully on the Ubuntu 16.04 machine. However, the tests would not run on the Windows 8.1 machine unless line 35 was changed to use phantomjs.cmd instead of phantomjs.exe.

I am using the phantomjs-prebuiltnpm module.

[dflynn15]

@MNR1 pushed up a fix. I forgot to rename the function after I had fixed it. My mistake 🙈

[gwynjudd]

I gave this a try and I still get the same ENOENT issue as before. I think that what is going on is that on some windows systems (if you did npm install phantomjs-prebuilt -g), you will have a phantomjs.cmd in your PATH. On other systems (like mine, if you downloaded phantomjs directly), you will have phantomjs in your PATH. Are you planning on supporting both? If so, then hasGlobalPhantom needs to be smarter about checking that:

function hasGlobalPhantom() {
  if(process.platform === 'win32') {
    try {
      exec('where phantomjs');
    } catch (e) {
      return false;
    }
  } else {
    try {
      exec('which phantomjs');
    } catch (e) {
      return false;
    }
  }
  return true;
}

Also getExecuteablePath would also be wrong:

function getExecuteablePath() {
  if (hasGlobalPhantom()) {
    return process.platform === 'win32' ? 'phantomjs.cmd' : 'phantomjs';
  }

  return require('phantomjs').path;
}

Maybe the correct solution is to have a configuration option for the caller to override the phantomjs path (so that they could use phantomjs-prebuilt if they wanted to, or have it installed globally somewhere not in the path or however they liked).

[dflynn15]

I can definitely modify those functions to do a quick check for both phantomjs and phantomjs.cmd. I'm still a bit baffled as to why some Windows machines have different executables.

Doing a quick check for either phantomjs.cmd or phantomjs seems like an easier experience during setup rather than having an option. It would at least cut down on the amount of issues that would be logged for people who don't read the README all the way through ¯\_(ツ)_/¯.

I'll try and patch that up sometime today and move all of this "what version/where is Phantom" stuff to a different file for sanities sake.

[mnmercer]

So I looked into it a bit more and the .cmd file is a wrapper that node will create for libraries with binaries. It seems to create this file regardless of whether it was installed globally, i.e. with -g, or not on a Windows machine. Funny thing is on the command line, both phantomjs and phantomjs.cmd execute fine, but phantomjs won't work within the plugin's script.

I think the simple check for the existence of phantomjs.cmd and phantomjs should be able to cover all cases.

[gwynjudd]

I'm still a bit baffled as to why some Windows machines have different executables.

@dflynn15 it's because if you download phantomjs from http://phantomjs.org/, then you only get the .exe, but if you install it via npm e.g., as part of phantomjs-prebuilt you get the .exe and the .cmd