Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 30 additions & 3 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,47 @@ import testingLibrary from "eslint-plugin-testing-library";
import noRelativeImportPaths from "eslint-plugin-no-relative-import-paths";
import unusedImports from "eslint-plugin-unused-imports";

import requireFutureBlockCapture from "./eslint/rules/require-future-block-capture.ts";

// ESLint 9 flat config, converted from .eslintrc.json. Same rule set: the file
// is longer because flat config spells out what `extends` and `env` used to imply.
export default tseslint.config(
{ ignores: ["build/**", "src/locales/**", "src/**/snapshots/*.ts", "**/*.d.ts"] },
{
ignores: [
"dist/**",
"build/**",
"node_modules/**",
"coverage/**",
"eslint.config.*",
"src/locales/**",
"src/**/snapshots/*.ts",
"**/*.d.ts",
],
},

js.configs.recommended,
...tseslint.configs.recommended,
react.configs.flat.recommended,
react.configs.flat["jsx-runtime"],
reactHooks.configs.flat.recommended,

{
languageOptions: {
parserOptions: { project: "./tsconfig.json" },
parserOptions: {
ecmaVersion: "latest",
project: "./tsconfig.json",
sourceType: "module",
},
},
settings: {
react: { pragma: "React", version: "16.6.0" },
react: { version: "detect" },
},
plugins: {
local: {
rules: {
"require-future-block-capture": requireFutureBlockCapture,
},
},
"react-hooks": reactHooks,
"no-relative-import-paths": noRelativeImportPaths,
"unused-imports": unusedImports,
Expand Down Expand Up @@ -77,11 +101,14 @@ export default tseslint.config(

"react-hooks/rules-of-hooks": "warn",
"react-hooks/exhaustive-deps": "warn",
// TODO: Enable `react-hooks/set-state-in-effect`.
"react-hooks/set-state-in-effect": "off",

"no-relative-import-paths/no-relative-import-paths": [
"error",
{ allowSameFolder: true, rootDir: "src", prefix: "$" },
],
"local/require-future-block-capture": "error",
},
},

Expand Down
3 changes: 3 additions & 0 deletions eslint/rules/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
21 changes: 21 additions & 0 deletions eslint/rules/require-future-block-capture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# require-future-block-capture

Inside a `Future.block` callback, await a `Future` through the callback's capture function. This lets the block track the operation's cancellation and failure.

Incorrect:

```ts
Future.block(async $ => {
const user = await loadUser();
return user;
});
```

Correct, when `loadUserFuture()` returns a `Future`:

```ts
Future.block(async $ => {
const user = await $(loadUserFuture());
return user;
});
```
81 changes: 81 additions & 0 deletions eslint/rules/require-future-block-capture.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { TSESLint } from "@typescript-eslint/utils";
import { describe, expect, test } from "vitest";

import requireFutureBlockCapture from "./require-future-block-capture";

function lint(source: string) {
const linter = new TSESLint.Linter({ configType: "flat" });
const config = {
languageOptions: { ecmaVersion: 2022, sourceType: "module" },
plugins: {
local: {
rules: { "require-future-block-capture": requireFutureBlockCapture },
},
},
rules: { "local/require-future-block-capture": "error" },
} satisfies TSESLint.FlatConfig.Config;

return linter.verifyAndFix(source, config, {});
}

describe("require-future-block-capture", () => {
test("allows Futures captured in Future.block", () => {
const result = lint("Future.block(async $ => await $(Future.success(1))); ");

expect(result.messages).toEqual([]);
});

test("allows Futures captured in Future.block_", () => {
const result = lint("Future.block_()(async capture => await capture(Future.success(1))); ");

expect(result.messages).toEqual([]);
});

test("reports but does not rewrite a native Promise", () => {
const source = "Future.block(async $ => await Promise.resolve(1));";
const result = lint(source);

expect(result.output).toBe(source);
expect(result.messages).toMatchObject([{ messageId: "wrapAwait" }]);
});

test("does not report awaits in nested function declarations", () => {
const source = `
Future.block(async $ => {
async function helper() {
await fetch(url);
}

return $(Future.fromPromise(helper()));
});
`;

expect(lint(source).messages).toEqual([]);
});

test("does not report awaits in nested arrow functions", () => {
const source = `
Future.block(async $ => {
const helper = async () => {
await fetch(url);
};

return $(Future.fromPromise(helper()));
});
`;

expect(lint(source).messages).toEqual([]);
});

test("reports awaits when the callback has no capture parameter", () => {
const result = lint("Future.block(async () => await Promise.resolve(1));");

expect(result.messages).toMatchObject([{ messageId: "wrapAwait" }]);
});

test("reports awaits when the callback destructures its capture parameter", () => {
const result = lint("Future.block(async ({ length }) => await Promise.resolve(length));");

expect(result.messages).toMatchObject([{ messageId: "wrapAwait" }]);
});
});
96 changes: 96 additions & 0 deletions eslint/rules/require-future-block-capture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { ESLintUtils, type TSESLint, type TSESTree } from "@typescript-eslint/utils";

type FunctionNode =
| TSESTree.ArrowFunctionExpression
| TSESTree.FunctionDeclaration
| TSESTree.FunctionExpression;

function isIdentifier(
node: TSESTree.Node | null | undefined,
name: string
): node is TSESTree.Identifier {
return node?.type === "Identifier" && node.name === name;
}

function isFutureMethodCall(
node: TSESTree.Node | null | undefined,
method: "block" | "block_"
): boolean {
return (
node?.type === "CallExpression" &&
node.callee.type === "MemberExpression" &&
!node.callee.computed &&
isIdentifier(node.callee.object, "Future") &&
isIdentifier(node.callee.property, method)
);
}

function isFunctionNode(node: TSESTree.Node): node is FunctionNode {
return (
node.type === "ArrowFunctionExpression" ||
node.type === "FunctionDeclaration" ||
node.type === "FunctionExpression"
);
}

function getFunctionAncestor(
sourceCode: TSESLint.SourceCode,
node: TSESTree.Node
): FunctionNode | null {
return sourceCode.getAncestors(node).findLast(isFunctionNode) ?? null;
}

function isFutureBlockCallback(functionNode: FunctionNode): boolean {
const parent = functionNode.parent;
if (parent?.type !== "CallExpression" || parent.arguments[0] !== functionNode) return false;

return isFutureMethodCall(parent, "block") || isFutureMethodCall(parent.callee, "block_");
}

const createRule = ESLintUtils.RuleCreator(
name => `https://github.com/EyeSeeTea/dhis2-app-skeleton/blob/master/eslint/rules/${name}.md`
);

export default createRule({
name: "require-future-block-capture",
meta: {
type: "problem",
docs: {
description:
"Require await calls inside Future.block callbacks to go through the capture function",
},
schema: [],
messages: {
wrapAwait: "Use `await {{capture}}(...)` inside `Future.block`.",
},
},
defaultOptions: [],
create(context) {
const sourceCode = context.sourceCode;

return {
AwaitExpression(node) {
const functionNode = getFunctionAncestor(sourceCode, node);
if (!functionNode) return;
if (!isFutureBlockCallback(functionNode)) return;

const firstParam = functionNode.params[0];
const captureName = firstParam?.type === "Identifier" ? firstParam.name : null;
const awaited = node.argument;
if (
captureName !== null &&
awaited.type === "CallExpression" &&
isIdentifier(awaited.callee, captureName)
) {
return;
}

context.report({
node,
messageId: "wrapAwait",
data: { capture: captureName ?? "$" },
});
},
};
},
});
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,14 @@
"@types/react-router-dom": "5.3.3",
"@typescript-eslint/eslint-plugin": "^8",
"@typescript-eslint/parser": "^8",
"@typescript-eslint/utils": "^8.65.0",
"@vitejs/plugin-react": "^5.0.4",
"cmd-ts": "^0",
"depcheck": "^1.4.7",
"eslint": "^9.39.5",
"eslint-plugin-no-relative-import-paths": "^1.5.3",
"eslint-plugin-react": "^7.37.5",
"eslint-plugin-react-hooks": "^5.2.0",
"eslint-plugin-react-hooks": "^7.1.1",
"eslint-plugin-testing-library": "^7.16.2",
"eslint-plugin-unused-imports": "^4",
"expect-type": "^0",
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@
"$/*": ["./*"]
}
},
"include": ["src"],
"include": ["src", "eslint/**/*.ts"],
"references": [{ "path": "./tsconfig.node.json" }]
}
Loading
Loading