Skip to content

Commit

Permalink
[[CHORE]] Relocate development dependencies
Browse files Browse the repository at this point in the history
The `phantom` and `phantomjs-prebuilt` modules cannot be installed in
legacy versions of Node.js because during their installation procedure,
they import code which uses ES2015 language features. When executed in
legacy versions of Node.js, this causes a syntax error, interrupting
installation and making it impossible to execute the project's tests.

A previous commit [1] addressed this issue by marking the dependencies
as "optional." This allowed the installation procedure to fail, and
because the packages are only necessary in the latest release of
Node.js, the relevant tests could execute as expected.

Unfortunately, this modification had an unintended effect on consumers
of JSHint: it forced them to also install the aforementioned packages.
Because these packages are only necessary when contributing to JSHint
(not when using it) and because they include large binary assets, this
had a non-trivial impact on the time and disk space required to install
JSHint.

Revert the previous change, marking the packages as "dev dependencies"
once again. Ensure that the project's tests can continue to execute in
legacy versions of Node.js by introducing a script that removes the
offending packages prior to installation on those platforms.

[1] 2f6ac13
  • Loading branch information
jugglinmike authored and rwaldron committed Aug 6, 2018
1 parent d5c1a00 commit f70250b
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 4 deletions.
7 changes: 7 additions & 0 deletions .travis.yml
@@ -1,5 +1,12 @@
language: node_js
after_success: npm run coverage && cat ./coverage/lcov.info | coveralls
before_install:
# These packages cannot be installed in legacy versions of Node.js. Because
# they are only needed to verify correctness in a browser-like environment,
# they can be removed (and the tests skipped) without effecting coverage.
- if [[ $(node --version) != v6* ]] ; then
node scripts/remove-dev-dependencies.js phantom phantomjs-prebuilt;
fi
script:
- npm run pretest
- npm run test-node
Expand Down
4 changes: 4 additions & 0 deletions appveyor.yml
Expand Up @@ -12,6 +12,10 @@ environment:
install:
- ps: Install-Product node $env:nodejs_version
- git submodule update --init --recursive
# These packages cannot be installed in legacy versions of Node.js. Because
# they are only needed to verify correctness in a browser-like environment,
# they can be removed (and the tests skipped) without effecting coverage.
- if NOT "%nodejs_version%" == "6" node scripts/remove-dev-dependencies.js phantom phantomjs-prebuilt
- npm install

build: off
Expand Down
6 changes: 2 additions & 4 deletions package.json
Expand Up @@ -59,16 +59,14 @@
"jscs": "1.11.x",
"mock-stdin": "0.3.x",
"nodeunit": "0.9.x",
"phantom": "~4.0.1",
"phantomjs-prebuilt": "~2.1.7",
"regenerate": "1.2.x",
"results-interpreter": "~1.0.0",
"sinon": "1.12.x",
"test262-stream": "~1.1.0",
"unicode-11.0.0": "0.7.x"
},
"optionalDependencies": {
"phantom": "~4.0.1",
"phantomjs-prebuilt": "~2.1.7"
},
"license": "(MIT AND JSON)",
"preferGlobal": true,
"files": [
Expand Down
37 changes: 37 additions & 0 deletions scripts/remove-dev-dependencies.js
@@ -0,0 +1,37 @@
"use strict"
/**
* Some of this project's development dependencies cannot be successfully
* installed in legacy versions of Node.js. This module removes development
* dependencies from the project's npm package manifest so that the `npm`
* utility does not attempt to install them in a subsequent execution of `npm
* install. Although the `npm` utility offers a "remove" command, that command
* can remove only one package per invocation and triggers installation of all
* other packages. This script allows mutliple packages to be removed prior to
* installation.
*/

var path = require("path");
var fs = require("fs");
var manifestLocation = path.join(__dirname, "..", "package.json");
var manifest = require(manifestLocation);
var packageNames = process.argv.slice(2);
var newContents;

packageNames.forEach(function(packageName) {
if (!manifest.devDependencies[packageName]) {
throw new Error("Could not locate development dependency named \"" + packageName + "\"");
}

delete manifest.devDependencies[packageName];
});

newContents = JSON.stringify(manifest, null, 2);

fs.writeFile(manifestLocation, newContents, function(err) {
if (err) {
throw new Error(err);
}

console.log("Successfully removed packages. New contents:");
console.log(newContents.replace(/(^|\n)/g, "$1> "));
});

0 comments on commit f70250b

Please sign in to comment.