Medium / phantomjs

NPM wrapper for installing phantomjs
Other
1.42k stars 436 forks source link

Merged PR#746 breaks npm install for node 0.12.x #753

Open JBlackCat opened 6 years ago

JBlackCat commented 6 years ago

For those of us stuck on node 0.12.x & npm 2.x.x

The following PR, https://github.com/Medium/phantomjs/pull/746, breaks npm i with the following error because it was released as a patch version.

/node_modules/phantomjs-prebuilt/node_modules/request/node_modules/hawk/node_modules/boom/lib/index.js:5
const Hoek = require('hoek');
^^^^^
SyntaxError: Use of const in strict mode.
JBlackCat commented 6 years ago

npm i phantomjs-prebuilt@2.1.15 works as expected.

mnicic commented 6 years ago

The same problem on 0.10.48. Shall we use shrinkwrap as the workaround or to wait for a bugfix?

JBlackCat commented 6 years ago

We still have a library that relies on npm 2 to install peerDependencies, so for my usage I added phantomjs-prebuilt as a peerDependency and hard stopped at 2.1.15 in the package.json by removing the ^. This fixed our issues when installing karma-phantomjs-launcher.

Additionally, adding a shrinkwrap file is best practice if you have a library that is widely used or critical.

mirkara commented 6 years ago

I too would like an answer to @mnicic 's question of using a workaround or waiting for a bugfix.

JBlackCat commented 6 years ago

You should be able to add

"phantomjs-prebuilt": "2.1.15"

To your devDependencies. Make sure you remove the ^, even if you are shrinkwrapped, or else if you are using something such as karma-phantomjs-launcher it will pull in the latest version.

Considering there hasn't been a response from the maintainers, I would patch for now.

JBlackCat commented 6 years ago

@mnicic and @mirkara, after some investigation a dependency change in version 2.1.15 broke npm install for node 0.10.x.

I created the following issue, https://github.com/Medium/phantomjs/issues/755

mnicic commented 6 years ago

I fixed this by locking the dependencies. But still, the incompatibility problem should be fixed. Thanks!