Install elm-json using elm-tooling (#480)

This PR uses [elm-tooling](https://github.com/lydell/elm-tooling.json/tree/main/cli) to install elm-json instead of the elm-json npm package.

Look at this size improvement:

| Version          |  Size |
| ---------------- | ----: |
| 0.19.1-revision3 | 19 MB |
| 0.19.1-revision4 | 18 MB |
| master           | 17 MB |
| This PR          |  2 MB |

Size was measured by running `du -sh node_modules` after installing node-test-runner (and nothing else) on MacOS.

The elm-json binary itself around 2 MB on MacOS.

The huge space drop comes from not depending on `binwrap` anymore. It has some pretty have dependencies.

Benefits of using elm-tooling:

- Faster installs. Fewer npm dependencies to install. elm-test users often use elm-test as well – once both use elm-tooling there won’t be a double elm-json download on each `npm install`. And if the user uses elm-tooling to install elm and elm-format as well, all three tools are downloaded in parallel. [Similar PR for elm-review.](jfmengels/node-elm-review#28)
- Less disk space wasted: Less dependencies in node_modules, and no copy of elm-json in every project.
- Security. elm-tooling uses sha256 to verify that it downloads what it expects.

The only noticeable difference is _when_ the elm-json executable is transferred from the Internet to the user’s machine.

- Previously, it happened during `npm install`. Unless you use [ignore-scripts](https://docs.npmjs.com/using-npm/config#ignore-scripts) – then it happened the first time `elm-json` was executed. Now, it happens right before `elm-json` is about to be executed. Unless the user already downloaded elm-json to `~/.elm/elm-tooling/` before.

- Given that node-test-runner executes both elm and elm-json, which both can make network requests, I think it’s fine to also execute elm-tooling’s `getExectuable` function which also can make network requests. node-test-runner continues to work offline once all needed things exist on the user’s machine.

- binwrap uses the [request](https://github.com/request/request) module, while elm-tooling uses curl if available, otherwise wget and finally Node.js’ [https](https://nodejs.org/api/https.html) module. If the user used [request’s environment variables for proxies](https://github.com/request/request#controlling-proxy-behaviour-using-environment-variables), that should probably just work since [curl seems to use the same environment variables](https://ec.haxx.se/usingcurl/usingcurl-proxies#proxy-environment-variables). But there might be some little difference. Either way, both curl and wget also support config files that lets the user alter how they work as needed, which is a good thing.

- Previously, the elm-json binary was downloaded to `./node_modules/elm-json/unpacked-bin/`. Now it’s downloaded to `~/.elm/elm-tooling/elm-json/0.2.8/`. Elm and elm-json write to `~/.elm` too, so it should be fine. (elm-tooling supports `ELM_HOME` and Windows.)

The plan now is to:

- Try to transfer `elm-tooling` to the [@elm-tooling](https://github.com/elm-tooling/) org so I don’t become a single point of failure.
- Release elm-tooling 1.0.0. It works on Linux, macOS and Windows, has 100% test coverage, has been tested by a couple of people on Discord (Dillon Kearns even put it in his [elm-pages-starter](https://github.com/dillonkearns/elm-pages-starter) template) and I haven’t found any improvements to make in a month.
- Merge this PR and [the one for elm-review](jfmengels/node-elm-review#28).
- Announce `elm-tooling` in Slack and Discourse. So far I haven’t talked much about it, waiting for these two PRs to be made.

.github/workflows/check.yml

@@ -36,11 +36,16 @@ jobs:
         uses: actions/cache@v2
         with:
           path: '${{ env.ELM_HOME }}'
-          key: elm-${{ matrix.os }}-${{ hashFiles('elm/**/elm.json', 'example-*/**/elm.json', 'tests/**/elm.json') }}
+          key: elm-${{ matrix.os }}-${{ hashFiles('elm-tooling.json', 'elm/**/elm.json', 'example-*/**/elm.json', 'tests/**/elm.json') }}
 
       - name: npm ci
         if: steps.cache-node_modules.outputs.cache-hit != 'true'
         run: npm ci
+        env:
+          NO_ELM_TOOLING_INSTALL: 1
+
+      - name: elm-tooling install
+        run: npx --no-install elm-tooling install
 
       - name: Flow
         run: npx --no-install flow check

.github/workflows/test.yml

@@ -36,11 +36,16 @@ jobs:
         uses: actions/cache@v2
         with:
           path: '${{ env.ELM_HOME }}'
-          key: elm-${{ matrix.os }}-${{ hashFiles('elm/**/elm.json', 'example-*/**/elm.json', 'tests/**/elm.json') }}
+          key: elm-${{ matrix.os }}-${{ hashFiles('elm-tooling.json', 'elm/**/elm.json', 'example-*/**/elm.json', 'tests/**/elm.json') }}
 
       - name: npm ci
         if: steps.cache-node_modules.outputs.cache-hit != 'true'
         run: npm ci
+        env:
+          NO_ELM_TOOLING_INSTALL: 1
+
+      - name: elm-tooling install
+        run: npx --no-install elm-tooling install
 
       - name: Mocha
         run: npx --no-install mocha tests

elm-tooling.json

@@ -0,0 +1,7 @@
+{
+    "tools": {
+        "elm": "0.19.1",
+        "elm-format": "0.8.4",
+        "elm-json": "0.2.8"
+    }
+}

lib/ElmCompiler.js

@@ -99,7 +99,7 @@ function compile(
       throw err;
     });
   } catch (err) {
-    throw compilerErrorToString(err, optionsWithDefaults.pathToElm);
+    throw new Error(compilerErrorToString(err, optionsWithDefaults.pathToElm));
   }
 }
 

lib/Generate.js

@@ -64,7 +64,10 @@ function getGeneratedSrcDir(generatedCodeDir /*: string */) /*: string */ {
   return path.join(generatedCodeDir, 'src');
 }
 
-function generateElmJson(project /*: typeof Project.Project */) /*: void */ {
+async function generateElmJson(
+  project /*: typeof Project.Project */,
+  onSolveProgress /*: typeof Solve.OnProgress */
+) /*: Promise<void> */ {
   const generatedSrc = getGeneratedSrcDir(project.generatedCodeDir);
 
   fs.mkdirSync(generatedSrc, { recursive: true });
@@ -95,7 +98,7 @@ function generateElmJson(project /*: typeof Project.Project */) /*: void */ {
     type: 'application',
     'source-directories': sourceDirs,
     'elm-version': '0.19.1',
-    dependencies: Solve.getDependenciesCached(project),
+    dependencies: await Solve.getDependenciesCached(project, onSolveProgress),
     'test-dependencies': {
       direct: {},
       indirect: {},

lib/RunTests.js

@@ -3,6 +3,7 @@
 const chalk = require('chalk');
 const chokidar = require('chokidar');
 const path = require('path');
+const readline = require('readline');
 const packageInfo = require('../package.json');
 const Compile = require('./Compile');
 const ElmJson = require('./ElmJson');
@@ -12,8 +13,6 @@ const Project = require('./Project');
 const Report = require('./Report');
 const Supervisor = require('./Supervisor');
 
-void Report;
-
 // Incorporate the process PID into the socket name, so elm-test processes can
 // be run parallel without accidentally sharing each others' sockets.
 //
@@ -29,21 +28,56 @@ function getPipeFilename(runsExecuted /*: number */) /*: string */ {
     : `/tmp/elm_test-${process.pid}.sock`;
 }
 
-function infoLog(
-  report /*: typeof Report.Report */,
-  msg /*: string */
-) /*: void */ {
-  if (report === 'console') {
-    console.log(msg);
-  }
-}
-
-function clearConsole(report /*: typeof Report.Report */) {
-  if (report === 'console') {
-    process.stdout.write(
-      process.platform === 'win32' ? '\x1B[2J\x1B[0f' : '\x1B[2J\x1B[3J\x1B[H'
-    );
-  }
+// This could have been a class, but I couldn’t figure out how to type it with
+// Flow comments and Prettier.
+// This lets you log something like `elm.json changed > Compiling > Starting tests`
+// where each segment appears over time.
+// `\r` moves the cursor to the start of the line. This is important when there
+// are Elm compilation errors - they overwrite the progress text rather than
+// weirdly ending up with stuff like:
+// `elm.json changed > Compiling-- NAMING ERROR - File.elm`
+// Also note that using too much `clearLine` or `clearConsole` causes flickering
+// on Windows, so it's nicer to cleverly overwrite old output when possible.
+function makeProgressLogger(report /*: typeof Report.Report */) {
+  const items = [];
+  return {
+    log(message) {
+      items.push(message);
+      if (!Report.isMachineReadable(report)) {
+        process.stdout.write(`${items.join(' > ')}\r`);
+      }
+    },
+    logLine(message) {
+      this.log(message);
+      items.length = 0;
+      if (!Report.isMachineReadable(report)) {
+        process.stdout.write('\n');
+      }
+    },
+    overwrite(message) {
+      items.length = 0;
+      items.push(message);
+      if (!Report.isMachineReadable(report)) {
+        process.stdout.write(`${message}\r`);
+      }
+    },
+    clearLine() {
+      items.length = 0;
+      if (!Report.isMachineReadable(report)) {
+        readline.clearLine(process.stdout, 0);
+      }
+    },
+    clearConsole() {
+      items.length = 0;
+      if (!Report.isMachineReadable(report)) {
+        process.stdout.write(
+          process.platform === 'win32'
+            ? '\x1B[2J\x1B[0f'
+            : '\x1B[2J\x1B[3J\x1B[H'
+        );
+      }
+    },
+  };
 }
 
 function diffArrays/*:: <T> */(
@@ -69,16 +103,14 @@ const Queue /*: Array<{
 void Queue;
 
 function watcherEventMessage(queue /*: typeof Queue */) /*: string */ {
-  const suffix = '. Rebuilding!';
-
   const filePaths = Array.from(new Set(queue.map(({ filePath }) => filePath)));
   if (filePaths.length === 1) {
     const { event, filePath } = queue[0];
-    return `${filePath} ${event}${suffix}`;
+    return `${filePath} ${event}`;
   }
 
   const events = Array.from(new Set(queue.map(({ event }) => event))).sort();
-  return `${filePaths.length} files ${events.join('/')}${suffix}`;
+  return `${filePaths.length} files ${events.join('/')}`;
 }
 
 function runTests(
@@ -104,6 +136,8 @@ function runTests(
   let currentRun /*: Promise<void> | void */ = undefined;
   let queue /*: typeof Queue */ = [];
 
+  const progressLogger = makeProgressLogger(report);
+
   async function run() /*: Promise<number> */ {
     try {
       // Don’t delay the first run (that’s the only time the queue is empty).
@@ -121,11 +155,14 @@ function runTests(
       // I tested touching 100 files at a time. All of them produced events
       // within 60 ms on Windows, and faster on Mac and Linux. So 100 ms sounds
       // like a reasonable number – not too short, not too long of a wait.
-      if (queue.length > 0) {
+      const queueLengthBeforeWaiting = queue.length;
+      if (queueLengthBeforeWaiting > 0) {
         await delay(100);
         // Re-print the message in case the queue has become longer while waiting.
-        clearConsole(report);
-        infoLog(report, watcherEventMessage(queue));
+        if (queue.length > queueLengthBeforeWaiting) {
+          progressLogger.clearLine();
+          progressLogger.log(watcherEventMessage(queue));
+        }
       }
 
       queue = [];
@@ -164,7 +201,23 @@ function runTests(
       const mainModule = Generate.getMainModule(project.generatedCodeDir);
       const dest = path.join(project.generatedCodeDir, 'elmTestOutput.js');
 
-      Generate.generateElmJson(project);
+      await Generate.generateElmJson(project, (progress) => {
+        switch (progress.tag) {
+          case 'Download elm-json':
+            if (progress.percentage === 0) {
+              progressLogger.clearLine();
+            }
+            progressLogger.overwrite(
+              `Downloading elm-json ${Math.round(progress.percentage * 100)}%`
+            );
+            return null;
+          case 'Run elm-json':
+            progressLogger.log('Solving dependencies');
+            return null;
+        }
+      });
+
+      progressLogger.log('Compiling');
 
       Generate.generateMainModule(
         fuzz,
@@ -187,6 +240,8 @@ function runTests(
 
       Generate.prepareCompiledJsFile(pipeFilename, dest);
 
+      progressLogger.logLine('Starting tests');
+
       return await Supervisor.run(
         packageInfo.version,
         pipeFilename,
@@ -196,22 +251,24 @@ function runTests(
         watch
       );
     } catch (err) {
+      progressLogger.logLine('');
       console.error(err.message);
       return 1;
     }
   }
 
   if (watch) {
-    clearConsole(report);
-    infoLog(report, 'Running in watch mode');
+    progressLogger.clearConsole();
 
     const onRunFinish = () => {
       if (queue.length > 0) {
-        clearConsole(report);
-        infoLog(report, watcherEventMessage(queue));
+        progressLogger.clearConsole();
+        progressLogger.log(watcherEventMessage(queue));
         currentRun = run().then(onRunFinish);
       } else {
-        infoLog(report, chalk.blue('Watching for changes...'));
+        if (!Report.isMachineReadable(report)) {
+          console.log(chalk.blue('Watching for changes...'));
+        }
         currentRun = undefined;
       }
     };
@@ -234,8 +291,8 @@ function runTests(
           filePath: path.relative(projectRootDir, absoluteFilePath),
         });
         if (currentRun === undefined) {
-          clearConsole(report);
-          infoLog(report, watcherEventMessage(queue));
+          progressLogger.clearConsole();
+          progressLogger.log(watcherEventMessage(queue));
           currentRun = run().then(onRunFinish);
         }
       }

lib/Solve.js

@@ -2,21 +2,31 @@
 
 const spawn = require('cross-spawn');
 const crypto = require('crypto');
+const getExecutable = require('elm-tooling/getExecutable');
 const fs = require('fs');
 const path = require('path');
 const ElmJson = require('./ElmJson');
 const Project = require('./Project');
 
 void Project;
 
-function sha1(string) {
-  return crypto.createHash('sha1').update(string).digest('hex');
+function sha256(string) {
+  return crypto.createHash('sha256').update(string).digest('hex');
 }
 
-function getDependenciesCached(
-  project /*: typeof Project.Project */
-) /*: typeof ElmJson.DirectAndIndirectDependencies */ {
-  const hash = sha1(
+// Poor man’s type alias. We can’t use /*:: type OnProgress = ... */ because of:
+// https://github.com/prettier/prettier/issues/2597
+// `null` is used instead of `void` to make Flow force implementations to
+// exhaustively switch on all variants of the parameter.
+const OnProgress /*: (
+  { tag: 'Download elm-json', percentage: number } | { tag: 'Run elm-json' }
+) => null */ = () => null;
+
+async function getDependenciesCached(
+  project /*: typeof Project.Project */,
+  onProgress /*: typeof OnProgress */
+) /*: Promise<typeof ElmJson.DirectAndIndirectDependencies> */ {
+  const hash = sha256(
     JSON.stringify({
       dependencies: project.elmJson.dependencies,
       'test-dependencies': project.elmJson['test-dependencies'],
@@ -38,7 +48,10 @@ function getDependenciesCached(
     }
   }
 
-  const dependencies = getDependencies(ElmJson.getPath(project.rootDir));
+  const dependencies = await getDependencies(
+    ElmJson.getPath(project.rootDir),
+    onProgress
+  );
 
   fs.writeFileSync(cacheFile, dependencies);
 
@@ -48,9 +61,19 @@ function getDependenciesCached(
   );
 }
 
-function getDependencies(elmJsonPath /*: string */) /*: string */ {
+async function getDependencies(
+  elmJsonPath /*: string */,
+  onProgress /*: typeof OnProgress */
+) /*: Promise<string> */ {
+  const toolAbsolutePath = await getExecutable({
+    name: 'elm-json',
+    version: '^0.2.8',
+    onProgress: (percentage) =>
+      onProgress({ tag: 'Download elm-json', percentage }),
+  });
+  onProgress({ tag: 'Run elm-json' });
   const result = spawn.sync(
-    'elm-json',
+    toolAbsolutePath,
     [
       'solve',
       '--test',
@@ -74,4 +97,7 @@ function getDependencies(elmJsonPath /*: string */) /*: string */ {
   return result.stdout;
 }
 
-module.exports = { getDependenciesCached };
+module.exports = {
+  OnProgress,
+  getDependenciesCached,
+};