Skip to content

Commit

Permalink
fix(eslint): better messaging and bug fix (#3950
Browse files Browse the repository at this point in the history
A few fixes for the eslint plugin. 

1. Fix error message to remove the leading `$` since that was deprecated
in 1.5
2. Provides more explicit error messaging 
3. Fixes a bug that could cause the rule to exit early if no global deps
were provided
4. Cleans up / standardizes tests and mocks

This should also fix #3943
  • Loading branch information
tknickman authored Feb 27, 2023
1 parent 6535d63 commit 953adf4
Show file tree
Hide file tree
Showing 34 changed files with 252 additions and 211 deletions.
2 changes: 1 addition & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ dist/

/examples/

packages/eslint-plugin-turbo/__tests__/fixtures
packages/eslint-plugin-turbo/__fixtures__
packages/create-turbo/templates
packages/turbo-tracing-next-plugin/test/with-mongodb-mongoose
crates/*/tests/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
// new style, global env dependency
"globalEnv": ["NEW_STYLE_GLOBAL_ENV_KEY", "$NEW_STYLE_GLOBAL_ENV_KEY"],
// old style, global env dependency (deprecated)
"globalDependencies": ["$GLOBAL_ENV_KEY"],
"pipeline": {
"test": {
"outputs": ["coverage/**"],
"dependsOn": ["^build"]
},
"lint": {
"outputs": []
},
"dev": {
"cache": false
},
"build": {
"outputs": ["dist/**", ".next/**", "!.next/.cache/**"],
// task level env var deps
"env": ["NEW_STYLE_ENV_KEY"],
// old task level env var deps (deprecated)
"dependsOn": ["^build", "$TASK_ENV_KEY", "$ANOTHER_ENV_KEY"]
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export default function docs() {
if (process.env.ENV_1 === undefined) {
return 'does not exist';
return "does not exist";
}
return 'exists'
return "exists";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"$schema": "https://turbo.build/schema.json",
"extends": ["//"],
"pipeline": {
"build": {
"env": ["ENV_3"]
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export default function web() {
if (!process.env.ENV_2) {
return 'bar';
return "bar";
}
return 'foo'
return "foo";
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export default function foo() {
if (!process.env.IS_SERVER) {
return 'bar';
return "bar";
}
return 'foo'
return "foo";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
process.env.NONEXISTENT;
process.env.CI;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"eslint-plugin-turbo": "../../"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
process.env.CI;
69 changes: 23 additions & 46 deletions packages/eslint-plugin-turbo/__tests__/cwd.test.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,18 @@
import fs from "fs";
import path from "path";
import JSON5 from "json5";
import { execSync } from "child_process";
import { Schema } from "turbo-types";
import { setupTestFixtures } from "turbo-test-utils";

describe("eslint settings check", () => {
beforeAll(() => {
const cwd = path.join(__dirname, "fixtures", "workspace");
execSync(`npm install`, { cwd });
});

afterAll(() => {
const nodeModulesDir = path.join(
__dirname,
"fixtures",
"workspace",
"node_modules"
);
fs.rmSync(nodeModulesDir, { force: true, recursive: true });
const { useFixture } = setupTestFixtures({
directory: path.join(__dirname, "../"),
});

it("does the right thing for peers", () => {
const cwd = path.join(__dirname, "fixtures", "workspace");
const { root: cwd } = useFixture({ fixture: "workspace" });
execSync(`npm install`, { cwd });

const configString = execSync(`eslint --print-config peer.js`, {
cwd,
encoding: "utf8",
Expand All @@ -33,7 +25,10 @@ describe("eslint settings check", () => {
});

it("does the right thing for child dirs", () => {
const cwd = path.join(__dirname, "fixtures", "workspace", "child");
const { root } = useFixture({ fixture: "workspace" });
execSync(`npm install`, { cwd: root });

const cwd = path.join(root, "child");
const configString = execSync(`eslint --print-config child.js`, {
cwd,
encoding: "utf8",
Expand All @@ -47,38 +42,18 @@ describe("eslint settings check", () => {
});

describe("eslint cache is busted", () => {
let turboJsonPath: string;
let originalString: string;

beforeAll(() => {
const cwd = path.join(__dirname, "fixtures", "workspace");
execSync(`npm install`, { cwd });

turboJsonPath = path.join(__dirname, "fixtures", "workspace", "turbo.json");
originalString = fs.readFileSync(turboJsonPath, { encoding: "utf8" });
});

afterEach(() => {
fs.writeFileSync(turboJsonPath, originalString);
});

afterAll(() => {
fs.writeFileSync(turboJsonPath, originalString);

const nodeModulesDir = path.join(
__dirname,
"fixtures",
"workspace",
"node_modules"
);
fs.rmSync(nodeModulesDir, { force: true, recursive: true });
const { useFixture } = setupTestFixtures({
directory: path.join(__dirname, "../"),
});

it("catches a lint error after changing config", () => {
expect.assertions(2);

// ensure that we populate the cache with a failure.
const cwd = path.join(__dirname, "fixtures", "workspace", "child");
const { root, readJson, write } = useFixture({ fixture: "workspace" });
execSync(`npm install`, { cwd: root });

const cwd = path.join(root, "child");
try {
execSync(`eslint --format=json child.js`, { cwd, encoding: "utf8" });
} catch (error: any) {
Expand All @@ -88,17 +63,19 @@ describe("eslint cache is busted", () => {
messages: [
{
message:
"$NONEXISTENT is not listed as a dependency in any turbo.json",
"NONEXISTENT is not listed as a dependency in turbo.json",
},
],
},
]);
}

// change the configuration
const turboJson = JSON5.parse(originalString);
turboJson.globalEnv = ["CI", "NONEXISTENT"];
fs.writeFileSync(turboJsonPath, JSON.stringify(turboJson, null, 2));
const turboJson = readJson<Schema>("turbo.json");
if (turboJson && "globalEnv" in turboJson) {
turboJson.globalEnv = ["CI", "NONEXISTENT"];
write("turbo.json", JSON5.stringify(turboJson, null, 2));
}

// test that we invalidated the eslint cache
const output = execSync(`eslint --format=json child.js`, {
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

1 comment on commit 953adf4

@vercel
Copy link

@vercel vercel bot commented on 953adf4 Feb 27, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.