Skip to content

Commit

Permalink
Throw instead of warning.
Browse files Browse the repository at this point in the history
Fixes #68
  • Loading branch information
Richard Feldman committed May 15, 2018
1 parent 76acd58 commit fbb37d4
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 46 deletions.
51 changes: 24 additions & 27 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ var temp = require("temp").track();
var findAllDependencies = require("find-elm-dependencies").findAllDependencies;

var defaultOptions = {
emitWarning: console.warn,
spawn: spawn,
cwd: undefined,
pathToElm: undefined,
Expand Down Expand Up @@ -39,7 +38,7 @@ function prepareOptions(options, spawnFn) {

function prepareProcessArgs(sources, options) {
var preparedSources = prepareSources(sources);
var compilerArgs = compilerArgsFromOptions(options, options.emitWarning);
var compilerArgs = compilerArgsFromOptions(options);

return ["make"].concat(preparedSources ? preparedSources.concat(compilerArgs) : compilerArgs);
}
Expand All @@ -65,15 +64,21 @@ function runCompiler(sources, options, pathToElm) {
return options.spawn(pathToElm, processArgs, processOpts);
}

function handleCompilerError(err, pathToElm) {
function compilerErrorToString(err, pathToElm) {
if ((typeof err === "object") && (typeof err.code === "string")) {
handleError(pathToElm, err);
switch (err.code) {
case "ENOENT":
return ("Could not find Elm compiler \"" + pathToElm + "\". Is it installed?")

case "EACCES":
return ("Elm compiler \"" + pathToElm + "\" did not have permission to run. Do you need to give it executable permissions?");

default:
return ("Error attempting to run Elm compiler \"" + pathToElm + "\":\n" + err);
}
} else {
console.error("Exception thrown when attempting to run Elm compiler " + JSON.stringify(pathToElm) + ":\n");
return ("Exception thrown when attempting to run Elm compiler " + JSON.stringify(pathToElm) + ":\n");
}
throw err;

process.exit(1);
}

function compileSync(sources, options) {
Expand All @@ -83,7 +88,7 @@ function compileSync(sources, options) {
try {
return runCompiler(sources, optionsWithDefaults, pathToElm);
} catch (err) {
handleCompilerError(err, pathToElm);
throw compilerErrorToString(err, pathToElm);
}
}

Expand All @@ -94,13 +99,9 @@ function compile(sources, options) {

try {
return runCompiler(sources, optionsWithDefaults, pathToElm)
.on('error', function(err) {
handleError(pathToElm, err);

process.exit(1);
});
.on('error', function(err) { throw(err); });
} catch (err) {
handleCompilerError(err, pathToElm);
throw compilerErrorToString(err, pathToElm);
}
}

Expand All @@ -123,7 +124,13 @@ function compileToString(sources, options){
options.output = info.path;
options.processOpts = { stdio: 'pipe' }

var compiler = compile(sources, options);
var compiler;

try {
compiler = compile(sources, options);
} catch(compileError) {
return reject(compileError);
}

compiler.stdout.setEncoding("utf8");
compiler.stderr.setEncoding("utf8");
Expand Down Expand Up @@ -163,19 +170,9 @@ function compileToStringSync(sources, options) {
return fs.readFileSync(file.path, {encoding: "utf8"});
}

function handleError(pathToElm, err) {
if (err.code === "ENOENT") {
console.error("Could not find Elm compiler \"" + pathToElm + "\". Is it installed?")
} else if (err.code === "EACCES") {
console.error("Elm compiler \"" + pathToElm + "\" did not have permission to run. Do you need to give it executable permissions?");
} else {
console.error("Error attempting to run Elm compiler \"" + pathToElm + "\":\n" + err);
}
}

// Converts an object of key/value pairs to an array of arguments suitable
// to be passed to child_process.spawn for elm-make.
function compilerArgsFromOptions(options, emitWarning) {
function compilerArgsFromOptions(options) {
return _.flatten(_.map(options, function(value, opt) {
if (value) {
switch(opt) {
Expand Down
26 changes: 13 additions & 13 deletions test/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,18 @@ describe("#compile", function() {
});
});

it("invokes custom `emitWarning`", function (done) {
it("throws when given an unrecognized argument", function () {
var opts = {
foo: "bar",
emitWarning: chai.spy(),
output: "/dev/null",
verbose: true,
cwd: fixturesDir
};
var compileProcess = compiler.compile(prependFixturesDir("Parent.elm"), opts);

compileProcess.on("exit", function(exitCode) {
var desc = "Expected emitWarning to have been called";
expect(opts.emitWarning, desc).to.have.been.called();
done();
});
expect(function() {
var compileProcess = compiler.compile(prependFixturesDir("Parent.elm"), opts);

}).to.throw();
});
});

Expand Down Expand Up @@ -98,18 +95,21 @@ describe("#compileToString", function() {
});
});

it("invokes custom `emitWarning`", function () {
it("Rejects the Promise when given an unrecognized argument like `yes`", function () {
var opts = {
foo: "bar",
emitWarning: chai.spy(),
verbose: true,
cwd: fixturesDir
};

var compilePromise = compiler.compileToString(prependFixturesDir("Parent.elm"), opts);

return compilePromise.then(function(err) {
var desc = "Expected emitWarning to have been called";
expect(opts.emitWarning, desc).to.have.been.called();
return new Promise(function(resolve, reject) {
return compilePromise.then(function() {
reject("Expected the compilation promise to be rejected due to the unrecognized compiler argument.");
}).catch(function() {
resolve();
});
});
});

Expand Down
11 changes: 5 additions & 6 deletions test/compileSync.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,16 @@ describe("#compileSync", function() {
expect(exitCode, desc).to.equal(1);
});

it("invokes custom `emitWarning`", function () {
it("throws when given an unrecognized argument like `yes`", function () {
var opts = {
foo: "bar",
emitWarning: chai.spy(),
yes: true,
output: "/dev/null",
verbose: true,
cwd: fixturesDir
};
var compileProcess = compiler.compileSync(prependFixturesDir("Parent.elm"), opts);

var desc = "Expected emitWarning to have been called";
expect(opts.emitWarning, desc).to.have.been.called();
expect(function() {
var compileProcess = compiler.compileSync(prependFixturesDir("Parent.elm"), opts);
}).to.throw();
});
});

2 comments on commit fbb37d4

@kachkaev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see steps towards closing #68 @rtfeldman! Are you planning to support elm 0.18 with this change? The reason why I'm curious is because when I try 5.0.0-alpha1 in jfairbank/run-elm#14, yes: true no longer works. This means that I'm either getting an issue with this option or a problem with process.exit() when trying to use async method 😅

@kachkaev
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UPD: I found a workaround in jfairbank/run-elm#14 by faking console.error and process.exit

Please sign in to comment.