diff --git a/.changeset/eslint-no-await-navigate-use-task.md b/.changeset/eslint-no-await-navigate-use-task.md new file mode 100644 index 00000000000..47f34b8c4df --- /dev/null +++ b/.changeset/eslint-no-await-navigate-use-task.md @@ -0,0 +1,5 @@ +--- +'eslint-plugin-qwik': patch +--- + +Add `no-await-navigate-in-use-task` ESLint rule to catch awaiting `useNavigate()` inside blocking `useTask$` callbacks. diff --git a/packages/eslint-plugin-qwik/index.ts b/packages/eslint-plugin-qwik/index.ts index 1d43733c3e4..8e8600ff267 100644 --- a/packages/eslint-plugin-qwik/index.ts +++ b/packages/eslint-plugin-qwik/index.ts @@ -11,6 +11,7 @@ import { unusedServer } from './src/unusedServer'; import { useMethodUsage } from './src/useMethodUsage'; import { validLexicalScope } from './src/validLexicalScope'; import { noAsyncPreventDefault } from './src/noAsyncPreventDefault'; +import { noAwaitNavigateInUseTask } from './src/noAwaitNavigateInUseTask'; import pkg from './package.json'; type Rules = NonNullable; @@ -28,6 +29,7 @@ const rules = { 'jsx-a': jsxAtag, 'no-use-visible-task': noUseVisibleTask, 'no-async-prevent-default': noAsyncPreventDefault, + 'no-await-navigate-in-use-task': noAwaitNavigateInUseTask, } satisfies Rules; const recommendedRulesLevels = { @@ -43,6 +45,7 @@ const recommendedRulesLevels = { 'qwik/jsx-a': 'warn', 'qwik/no-use-visible-task': 'warn', 'qwik/no-async-prevent-default': 'warn', + 'qwik/no-await-navigate-in-use-task': 'warn', } satisfies TSESLint.FlatConfig.Rules; const strictRulesLevels = { @@ -58,6 +61,7 @@ const strictRulesLevels = { 'qwik/jsx-a': 'error', 'qwik/no-use-visible-task': 'warn', 'qwik/no-async-prevent-default': 'warn', + 'qwik/no-await-navigate-in-use-task': 'warn', } satisfies TSESLint.FlatConfig.Rules; const configs = { diff --git a/packages/eslint-plugin-qwik/src/noAwaitNavigateInUseTask.ts b/packages/eslint-plugin-qwik/src/noAwaitNavigateInUseTask.ts new file mode 100644 index 00000000000..3096b64cf38 --- /dev/null +++ b/packages/eslint-plugin-qwik/src/noAwaitNavigateInUseTask.ts @@ -0,0 +1,198 @@ +import type { Rule } from 'eslint'; +import type { + ArrowFunctionExpression, + CallExpression, + Expression, + FunctionExpression, + Node, + Pattern, +} from 'estree'; + +const USE_TASK_CALLEES = new Set(['useTask$', 'useTaskQrl']); + +function isUseTaskCall(node: CallExpression): boolean { + return node.callee.type === 'Identifier' && USE_TASK_CALLEES.has(node.callee.name); +} + +function getTaskCallback( + node: CallExpression +): ArrowFunctionExpression | FunctionExpression | null { + const arg0 = node.arguments[0]; + if (arg0?.type === 'ArrowFunctionExpression' || arg0?.type === 'FunctionExpression') { + return arg0; + } + return null; +} + +function isDeferUpdatesFalse(node: Expression | Pattern | undefined): boolean { + if (!node) { + return false; + } + if (node.type === 'AssignmentPattern') { + return isDeferUpdatesFalse(node.right); + } + if (node.type === 'Literal' && node.value === false) { + return true; + } + if (node.type === 'TSAsExpression') { + return isDeferUpdatesFalse(node.expression as Expression); + } + return false; +} + +function hasDeferUpdatesFalseOption(node: CallExpression): boolean { + const opts = node.arguments[1]; + if (!opts || opts.type !== 'ObjectExpression') { + return false; + } + for (const prop of opts.properties) { + if (prop.type !== 'Property' || prop.computed) { + continue; + } + const key = prop.key; + const name = + key.type === 'Identifier' ? key.name : key.type === 'Literal' ? String(key.value) : null; + if (name !== 'deferUpdates') { + continue; + } + if (isDeferUpdatesFalse(prop.value as Expression | Pattern)) { + return true; + } + } + return false; +} + +function collectUseNavigateBoundNamesFromNode(root: Node): Set { + const ids = new Set(); + const stack: Node[] = [root]; + while (stack.length) { + const n = stack.pop()!; + if (n.type === 'VariableDeclarator' && n.id.type === 'Identifier' && n.init) { + if ( + n.init.type === 'CallExpression' && + n.init.callee.type === 'Identifier' && + n.init.callee.name === 'useNavigate' + ) { + ids.add(n.id.name); + } + } + for (const key of Object.keys(n) as (keyof Node)[]) { + if (key === 'parent') { + continue; + } + const child = (n as unknown as Record)[key as string]; + if (Array.isArray(child)) { + for (const c of child) { + if (c && typeof c === 'object' && c !== null && 'type' in (c as object)) { + stack.push(c as Node); + } + } + } else if (child && typeof child === 'object' && 'type' in (child as object)) { + stack.push(child as Node); + } + } + } + return ids; +} + +function collectNavigateBindingsForUseTask(useTaskCall: CallExpression): Set { + const ids = new Set(); + let current: Node | null = useTaskCall.parent; + while (current) { + if (current.type === 'ArrowFunctionExpression' || current.type === 'FunctionExpression') { + const p = current.parent; + if ( + p?.type === 'CallExpression' && + p.callee.type === 'Identifier' && + p.callee.name === 'component$' + ) { + for (const id of collectUseNavigateBoundNamesFromNode(current)) { + ids.add(id); + } + } + } + if (current.type === 'Program') { + for (const id of collectUseNavigateBoundNamesFromNode(current)) { + ids.add(id); + } + break; + } + current = current.parent as Node | null; + } + return ids; +} + +function reportAwaitedNavigateCalls( + context: Rule.RuleContext, + root: Node, + navigateIds: Set +) { + const stack: Node[] = [root]; + while (stack.length) { + const n = stack.pop()!; + if (n.type === 'AwaitExpression') { + const arg = n.argument; + if (arg.type === 'CallExpression' && arg.callee.type === 'Identifier') { + if (navigateIds.has(arg.callee.name)) { + context.report({ + node: n, + messageId: 'noAwaitBlocking', + data: { name: arg.callee.name }, + }); + } + } + } + for (const key of Object.keys(n) as (keyof Node)[]) { + if (key === 'parent') { + continue; + } + const child = (n as unknown as Record)[key as string]; + if (Array.isArray(child)) { + for (const c of child) { + if (c && typeof c === 'object' && c !== null && 'type' in (c as object)) { + stack.push(c as Node); + } + } + } else if (child && typeof child === 'object' && 'type' in (child as object)) { + stack.push(child as Node); + } + } + } +} + +export const noAwaitNavigateInUseTask: Rule.RuleModule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow awaiting the function returned by `useNavigate()` inside blocking `useTask$` callbacks.', + recommended: true, + url: 'https://qwik.dev/docs/advanced/eslint/', + }, + messages: { + noAwaitBlocking: + 'Awaiting `{{name}}()` from `useNavigate()` inside a blocking `useTask$` can deadlock. Remove `await`, or pass `{ deferUpdates: false }` as the second argument to `useTask$`.', + }, + }, + create(context) { + return { + CallExpression(node: CallExpression) { + if (!isUseTaskCall(node)) { + return; + } + if (hasDeferUpdatesFalseOption(node)) { + return; + } + const taskFn = getTaskCallback(node); + if (!taskFn) { + return; + } + const navigateIds = collectNavigateBindingsForUseTask(node); + if (!navigateIds.size) { + return; + } + reportAwaitedNavigateCalls(context, taskFn.body, navigateIds); + }, + }; + }, +}; diff --git a/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/invalid-blocking.tsx b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/invalid-blocking.tsx new file mode 100644 index 00000000000..126a9582175 --- /dev/null +++ b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/invalid-blocking.tsx @@ -0,0 +1,12 @@ +// Expect error: { "messageId": "noAwaitBlocking" } + +import { component$, useTask$ } from '@builder.io/qwik'; +import { useNavigate } from '@builder.io/qwik-city'; + +export default component$(() => { + const nav = useNavigate(); + useTask$(async () => { + await nav('/'); + }); + return
; +}); diff --git a/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-defer-updates.tsx b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-defer-updates.tsx new file mode 100644 index 00000000000..dc354d5788f --- /dev/null +++ b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-defer-updates.tsx @@ -0,0 +1,13 @@ +import { component$, useTask$ } from '@builder.io/qwik'; +import { useNavigate } from '@builder.io/qwik-city'; + +export default component$(() => { + const nav = useNavigate(); + useTask$( + async () => { + await nav('/'); + }, + { deferUpdates: false } + ); + return
; +}); diff --git a/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-no-await.tsx b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-no-await.tsx new file mode 100644 index 00000000000..0a1cbbdc4cf --- /dev/null +++ b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-no-await.tsx @@ -0,0 +1,10 @@ +import { component$, useTask$ } from '@builder.io/qwik'; +import { useNavigate } from '@builder.io/qwik-city'; + +export default component$(() => { + const nav = useNavigate(); + useTask$(async () => { + void nav('/'); + }); + return
; +}); diff --git a/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-other-await.tsx b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-other-await.tsx new file mode 100644 index 00000000000..e2b19845788 --- /dev/null +++ b/packages/eslint-plugin-qwik/tests/no-await-navigate-in-use-task/valid-other-await.tsx @@ -0,0 +1,12 @@ +import { component$, useTask$ } from '@builder.io/qwik'; +import { useNavigate } from '@builder.io/qwik-city'; + +export default component$(() => { + const nav = useNavigate(); + const other = async () => Promise.resolve(); + useTask$(async () => { + await other(); + void nav('/'); + }); + return
; +});