From f70250bf584a3661ecddbbc23f83173a7b09c248 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Sat, 4 Aug 2018 20:41:31 -0400 Subject: [PATCH] [[CHORE]] Relocate development dependencies 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] 2f6ac131c3cd19cb3ddff12dd22e0ddda61ea7d8 --- .travis.yml | 7 ++++++ appveyor.yml | 4 ++++ package.json | 6 ++--- scripts/remove-dev-dependencies.js | 37 ++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 scripts/remove-dev-dependencies.js diff --git a/.travis.yml b/.travis.yml index 2223daadff..6e68b9c382 100644 --- a/.travis.yml +++ b/.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 diff --git a/appveyor.yml b/appveyor.yml index d338bfc210..7ee4f33693 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -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 diff --git a/package.json b/package.json index d926055486..7e57ab3fe9 100644 --- a/package.json +++ b/package.json @@ -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": [ diff --git a/scripts/remove-dev-dependencies.js b/scripts/remove-dev-dependencies.js new file mode 100644 index 0000000000..7b5149b543 --- /dev/null +++ b/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> ")); +});