Add phantomjs as a dependency

[NickTomlin]

Currently npm install fails for any project that includes phantomjssmith as a dependency when phantom isn't available globally (due to the preinstall hook). Since you already use which it should pick up and use a local phantom install, and it makes sense to add this as a dependency of the project.

I'll happily PR this change if it's welcome. Also, forgive me if i've missed something crucial here :)

[twolfson]

Initially this was introduced to prevent undesired installs on unsupported systems in spritesmith@0.x.x. At that time, we were installing all engines via optionalDependencies so any of them would be discarded if their requirements weren't met. We didn't install phantomjs as a dependency because of the large weight during each installation/reinstallation.

twolfson/spritesmith#27

Now that engines are opt-in, there are a few ways to approach this (not necessarily all the ways):

• Research if moving to a postinstall hook will cause npm install have a non-zero exit code (signifying failure)
  • This would allow projects to define the
• Add phantomjs as a dependency (your proposal)
  • Not preferred since this adds undesired weight to people who already have PhantomJS installed globally
• Add phantomjs as a postinstall fallback
  • Not preferred since this leads to inconsistent installs on machines (e.g. one person has ours, another has a global install)
• Create phantomjssmith module that comes bundled with phantomjs module and phantomjssmith
  • Depends on postinstall research

I am currently busy at the moment but will take a look by the end of the next weekend. At this point, I am leaning towards moving from preinstall to postinstall.

[NickTomlin]

Thanks for the in-depth response!

I understand the desire to reduce weight by depending on Phantom, but i'd like to continue to advocate for including it as a dependency the following reasons:

1. It avoids the need for install hooks, any breakage that can result from them, and the need for consumers to document the global phantom dependency.
2. It makes the module more predictable, since you'll be running of the version of phantom you pin instead of having to depend on the end user to have a compatible version of phantom installed globally
3. It's a common pattern. There are plenty of packages that depend on phantom (most testing frameworks out there). I don't think the hit is to severe, and users often have the download cached anyway.

Functionally, this would mean using the path from which wherever phantom is used, but I don't think that's too serious a change anyway.

Regardless, I think you are spot on in wanting to find a way to exit successfully from an install hook. I'll see if I can do some investigation into this on my end as well.

[twolfson]

It looks like postinstall works equally as well. I have chosen a middle ground to document both global and local installation in the README:

https://github.com/twolfson/phantomjssmith/tree/0.6.0#requirements

Thanks for the bug report!

[twolfson]

For what it's worth, if anyone still wants them to be bundled, then we can create a module which wraps the 2 together. However, I want to make it easier to opt-in to downloading additional content than making it difficult to opt-out.

[NickTomlin]

@twolfson awesome. Thanks so much for looking into this.