Skip to content

Commit cc8bb85

Browse files
author
Josh Goldberg
authored
Intentionally fail fast on module lookup failures (#419)
* Intentionally fail fast on module lookup failures * Added explicit test for TSLint failing to resolve * Teeny module ordering nit
1 parent bb06c71 commit cc8bb85

File tree

2 files changed

+86
-12
lines changed

2 files changed

+86
-12
lines changed

src/input/findOriginalConfigurations.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ const createStubDependencies = (
4747
});
4848

4949
describe("findOriginalConfigurations", () => {
50-
it("returns errors when the tslint finder returns an error", async () => {
50+
it("returns the error when the tslint finder returns an error", async () => {
5151
// Arrange
5252
const complaint = "Complaint from TSLint";
5353
const dependencies = createStubDependencies({
@@ -64,6 +64,23 @@ describe("findOriginalConfigurations", () => {
6464
});
6565
});
6666

67+
it("returns a package install error when the tslint finder returns a package install error", async () => {
68+
// Arrange
69+
const complaint = "could not require 'tslint'";
70+
const dependencies = createStubDependencies({
71+
findTSLintConfiguration: async () => new Error(complaint),
72+
});
73+
74+
// Act
75+
const result = await findOriginalConfigurations(dependencies, createRawSettings());
76+
77+
// Assert
78+
expect(result).toEqual({
79+
complaints: ["Could not import the 'tslint' module. Do you need to install packages?"],
80+
status: ResultStatus.ConfigurationError,
81+
});
82+
});
83+
6784
it("returns only tslint results when the other finders return errors", async () => {
6885
// Arrange
6986
const dependencies = createStubDependencies({
@@ -90,6 +107,33 @@ describe("findOriginalConfigurations", () => {
90107
});
91108
});
92109

110+
it.each(["Cannot find module", "could not require", "couldn't find the plugin"])(
111+
"returns an error when an optional configuration returns a '%s' error",
112+
async (message) => {
113+
// Arrange
114+
const eslint = new Error(`${message} "example"`);
115+
const dependencies = createStubDependencies({
116+
findESLintConfiguration: async () => eslint,
117+
});
118+
119+
// Act
120+
const result = await findOriginalConfigurations(
121+
dependencies,
122+
createRawSettings({
123+
eslint: "./eslintrc.js",
124+
}),
125+
);
126+
127+
// Assert
128+
expect(result).toEqual({
129+
complaints: [
130+
`Could not import the "example" module. Do you need to install packages?`,
131+
],
132+
status: ResultStatus.ConfigurationError,
133+
});
134+
},
135+
);
136+
93137
it("returns an error when an optional configuration returns an error and the user asked for it", async () => {
94138
// Arrange
95139
const eslint = new Error("one");

src/input/findOriginalConfigurations.ts

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { SansDependencies } from "../binding";
22
import { ResultStatus, TSLintToESLintSettings, ResultWithDataStatus } from "../types";
3+
import { isDefined } from "../utils";
34
import { findESLintConfiguration, ESLintConfiguration } from "./findESLintConfiguration";
45
import { PackagesConfiguration, findPackagesConfiguration } from "./findPackagesConfiguration";
56
import {
@@ -54,20 +55,42 @@ export const findOriginalConfigurations = async (
5455
// Out of those configurations, only TSLint's is always required to run
5556
if (tslint instanceof Error) {
5657
return {
57-
complaints: [tslint.message],
58+
complaints: [getMissingPackageMessage(tslint) ?? tslint.message],
5859
status: ResultStatus.ConfigurationError,
5960
};
6061
}
6162

62-
// Other configuration errors only halt the program if the user asked for them
63-
const configurationErrors = [
64-
[eslint, rawSettings.eslint],
65-
[packages, rawSettings.package],
66-
[typescript, rawSettings.typescript],
67-
].filter(configurationResultIsError);
68-
if (configurationErrors.length !== 0) {
63+
const configurationResults = [
64+
[eslint, "eslint"],
65+
[packages, "package"],
66+
[typescript, "typescript"],
67+
] as const;
68+
69+
// Other configuration errors only halt the program if...
70+
const errorMessages = configurationResults
71+
.map(([error, key]) => {
72+
if (!(error instanceof Error)) {
73+
return undefined;
74+
}
75+
76+
// * Their failure was caused by a missing package that needs to be installed
77+
const missingPackageMessage = getMissingPackageMessage(error);
78+
if (missingPackageMessage !== undefined) {
79+
return missingPackageMessage;
80+
}
81+
82+
// * The user explicitly asked for them
83+
if (typeof rawSettings[key] === "string") {
84+
return error.message;
85+
}
86+
87+
return undefined;
88+
})
89+
.filter(isDefined);
90+
91+
if (errorMessages.length !== 0) {
6992
return {
70-
complaints: configurationErrors.map(([configuration]) => configuration.message),
93+
complaints: errorMessages,
7194
status: ResultStatus.ConfigurationError,
7295
};
7396
}
@@ -83,6 +106,13 @@ export const findOriginalConfigurations = async (
83106
};
84107
};
85108

86-
const configurationResultIsError = (result: unknown[]): result is [Error, string] => {
87-
return result[0] instanceof Error && typeof result[1] === "string";
109+
const getMissingPackageMessage = (error: Error) => {
110+
const match = /(Cannot find module|could not require|couldn't find the plugin) ([a-zA-Z0-9-_"'@/]+)/.exec(
111+
error.message,
112+
);
113+
if (match === null) {
114+
return undefined;
115+
}
116+
117+
return `Could not import the ${match[2]} module. Do you need to install packages?`;
88118
};

0 commit comments

Comments
 (0)