Skip to content

Commit 26dd671

Browse files
committed
feat: extend eslint rule for route loaders
1 parent cc8a4cc commit 26dd671

File tree

3 files changed

+229
-73
lines changed

3 files changed

+229
-73
lines changed

packages/eslint-plugin-qwik/src/asyncComputedTop.ts

Lines changed: 169 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ import { Rule } from 'eslint';
22
import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/utils';
33
import ts from 'typescript';
44

5+
// Utility type to handle nodes from Rule handlers which may come from @types/estree
6+
// The nodes from Rule handlers are compatible with TSESTree at runtime but not at the type level
7+
// because @types/estree uses string literals while TSESTree uses AST_NODE_TYPES enum
8+
type RuleNode<T extends TSESTree.Node = TSESTree.Node> = {
9+
type: any;
10+
parent?: any;
11+
[key: string]: any;
12+
};
13+
514
function isFromQwikModule(resolvedVar: any): boolean {
615
return resolvedVar?.defs?.some((def: any) => {
716
if (def.type !== 'ImportBinding') {
@@ -87,7 +96,9 @@ function isAsyncComputedIdentifier(context: Rule.RuleContext, ident: any): boole
8796
const callee = decl.initializer.expression;
8897
if (ts.isIdentifier(callee)) {
8998
const name = callee.text;
90-
return name === 'createAsyncComputed$' || name === 'useAsyncComputed$';
99+
return (
100+
name === 'createAsyncComputed$' || name === 'useAsyncComputed$' || name === 'routeLoader$'
101+
);
91102
}
92103
}
93104
if (ts.isExportSpecifier(decl) || ts.isImportSpecifier(decl)) {
@@ -105,11 +116,39 @@ function isAsyncComputedIdentifier(context: Rule.RuleContext, ident: any): boole
105116
if (callee.type === AST_NODE_TYPES.Identifier) {
106117
const name = callee.name;
107118
if (
108-
(name === 'useAsyncComputed$' || name === 'createAsyncComputed$') &&
119+
(name === 'useAsyncComputed$' ||
120+
name === 'createAsyncComputed$' ||
121+
name === 'routeLoader$') &&
109122
isFromQwikModule(resolveVariableForIdentifier(context, callee))
110123
) {
111124
return true;
112125
}
126+
127+
// Check if callee was created by routeLoader$ (or other async computed creators)
128+
// e.g., const signal = useProductDetails() where useProductDetails = routeLoader$(...)
129+
const calleeResolved = resolveVariableForIdentifier(context, callee);
130+
if (calleeResolved && calleeResolved.defs) {
131+
for (const calleeDef of calleeResolved.defs) {
132+
if (
133+
calleeDef.type === 'Variable' &&
134+
calleeDef.node.type === AST_NODE_TYPES.VariableDeclarator
135+
) {
136+
const calleeInit = calleeDef.node.init;
137+
if (calleeInit && calleeInit.type === AST_NODE_TYPES.CallExpression) {
138+
const calleeCallee = calleeInit.callee;
139+
if (calleeCallee.type === AST_NODE_TYPES.Identifier) {
140+
const calleeName = calleeCallee.name;
141+
if (
142+
calleeName === 'routeLoader$' &&
143+
isFromQwikModule(resolveVariableForIdentifier(context, calleeCallee))
144+
) {
145+
return true;
146+
}
147+
}
148+
}
149+
}
150+
}
151+
}
113152
}
114153
}
115154
}
@@ -156,8 +195,8 @@ function isAsyncComputedIdentifier(context: Rule.RuleContext, ident: any): boole
156195
const tsNode = esTreeNodeToTSNodeMap.get(ident as any);
157196
const type = checker.getTypeAtLocation(tsNode);
158197
const typeStr = checker.typeToString(type.getNonNullableType());
159-
// Heuristic: type name includes AsyncComputed
160-
if (/AsyncComputed/i.test(typeStr)) {
198+
// Heuristic: type name includes AsyncComputed or LoaderSignal
199+
if (/AsyncComputed|LoaderSignal/i.test(typeStr)) {
161200
return true;
162201
}
163202
} catch {
@@ -168,9 +207,9 @@ function isAsyncComputedIdentifier(context: Rule.RuleContext, ident: any): boole
168207
return false;
169208
}
170209

171-
function hasAwaitResolveBefore(
210+
function hasAwaitPromiseBefore(
172211
body: TSESTree.BlockStatement,
173-
beforeStmt: TSESTree.Statement,
212+
beforeStmt: RuleNode<TSESTree.ExpressionStatement> | RuleNode<TSESTree.ReturnStatement>,
174213
identifierName: string
175214
): boolean {
176215
for (const stmt of body.body) {
@@ -184,23 +223,87 @@ function hasAwaitResolveBefore(
184223
if (expr.type !== AST_NODE_TYPES.AwaitExpression) {
185224
continue;
186225
}
187-
const awaited = expr.argument;
226+
let awaited = expr.argument;
227+
228+
// Handle `await signal.promise()`
229+
if (awaited.type === AST_NODE_TYPES.CallExpression) {
230+
awaited = awaited.callee;
231+
}
232+
233+
// Handle `await signal.promise`
188234
if (
189-
awaited &&
190-
awaited.type === AST_NODE_TYPES.CallExpression &&
191-
awaited.callee.type === AST_NODE_TYPES.MemberExpression &&
192-
!awaited.callee.computed &&
193-
awaited.callee.object.type === AST_NODE_TYPES.Identifier &&
194-
awaited.callee.object.name === identifierName &&
195-
awaited.callee.property.type === AST_NODE_TYPES.Identifier &&
196-
awaited.callee.property.name === 'promise'
235+
awaited.type === AST_NODE_TYPES.MemberExpression &&
236+
!awaited.computed &&
237+
awaited.object.type === AST_NODE_TYPES.Identifier &&
238+
awaited.object.name === identifierName &&
239+
awaited.property.type === AST_NODE_TYPES.Identifier &&
240+
awaited.property.name === 'promise'
197241
) {
198242
return true;
199243
}
200244
}
201245
return false;
202246
}
203247

248+
function findContainingFunction(
249+
node: RuleNode
250+
):
251+
| RuleNode<TSESTree.FunctionDeclaration>
252+
| RuleNode<TSESTree.FunctionExpression>
253+
| RuleNode<TSESTree.ArrowFunctionExpression>
254+
| null {
255+
let current: any = node;
256+
while (current) {
257+
if (
258+
current.type === AST_NODE_TYPES.FunctionDeclaration ||
259+
current.type === AST_NODE_TYPES.FunctionExpression ||
260+
current.type === AST_NODE_TYPES.ArrowFunctionExpression
261+
) {
262+
return current;
263+
}
264+
current = current.parent;
265+
}
266+
return null;
267+
}
268+
269+
function shouldCheckFunction(
270+
context: Rule.RuleContext,
271+
fn:
272+
| RuleNode<TSESTree.FunctionDeclaration>
273+
| RuleNode<TSESTree.FunctionExpression>
274+
| RuleNode<TSESTree.ArrowFunctionExpression>,
275+
signalIdent: RuleNode<TSESTree.Identifier>
276+
): boolean {
277+
// Check if this function is directly passed to a QRL (e.g., component$, $, etc.)
278+
const parent = fn.parent;
279+
if (parent && parent.type === AST_NODE_TYPES.CallExpression) {
280+
if (parent.callee.type === AST_NODE_TYPES.Identifier) {
281+
if (isQrlCallee(context, parent.callee)) {
282+
// Check if the function is in the arguments
283+
for (const arg of parent.arguments) {
284+
if (arg === fn) {
285+
return true;
286+
}
287+
}
288+
}
289+
}
290+
}
291+
292+
// For non-QRL functions, only check if:
293+
// 1. It's an async function
294+
// 2. The signal is a parameter (not captured from outer scope)
295+
if (fn.async) {
296+
// Check if the signal identifier is a parameter of this function
297+
for (const param of fn.params) {
298+
if (param.type === AST_NODE_TYPES.Identifier && param.name === signalIdent.name) {
299+
return true;
300+
}
301+
}
302+
}
303+
304+
return false;
305+
}
306+
204307
export const asyncComputedTop: Rule.RuleModule = {
205308
meta: {
206309
type: 'problem',
@@ -217,96 +320,89 @@ export const asyncComputedTop: Rule.RuleModule = {
217320
},
218321
},
219322
create(context) {
220-
const qrlFnStack: Array<
221-
TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression | TSESTree.FunctionDeclaration
222-
> = [];
223-
224-
function isInTrackedQrl(
225-
fn:
226-
| TSESTree.FunctionExpression
227-
| TSESTree.ArrowFunctionExpression
228-
| TSESTree.FunctionDeclaration
229-
): boolean {
230-
const parent = fn.parent;
231-
if (!parent || parent.type !== AST_NODE_TYPES.CallExpression) {
232-
return false;
233-
}
234-
if (parent.callee.type !== AST_NODE_TYPES.Identifier) {
235-
return false;
236-
}
237-
if (!isQrlCallee(context, parent.callee)) {
238-
return false;
239-
}
240-
// Function must be passed as a direct argument
241-
return parent.arguments.includes(fn as any);
242-
}
243-
244323
return {
245-
':function'(node: any) {
324+
MemberExpression(node) {
246325
if (
247-
(node.type === AST_NODE_TYPES.FunctionExpression ||
248-
node.type === AST_NODE_TYPES.ArrowFunctionExpression) &&
249-
isInTrackedQrl(node)
326+
node.computed ||
327+
node.property.type !== AST_NODE_TYPES.Identifier ||
328+
node.property.name !== 'value'
250329
) {
251-
qrlFnStack.push(node);
330+
return;
252331
}
253-
},
254-
':function:exit'(node: any) {
255-
if (qrlFnStack.length && qrlFnStack[qrlFnStack.length - 1] === node) {
256-
qrlFnStack.pop();
332+
333+
const obj = node.object;
334+
if (obj.type !== AST_NODE_TYPES.Identifier) {
335+
return;
257336
}
258-
},
259337

260-
MemberExpression(node) {
261-
if (!qrlFnStack.length) {
338+
if (!isAsyncComputedIdentifier(context, obj)) {
339+
return;
340+
}
341+
342+
const currentFn = findContainingFunction(node);
343+
if (!currentFn) {
344+
return;
345+
}
346+
347+
// Only check functions that are QRL callbacks or async functions with signal parameters
348+
if (!shouldCheckFunction(context, currentFn, obj)) {
262349
return;
263350
}
264-
const currentFn = qrlFnStack[qrlFnStack.length - 1]!;
351+
265352
// Only care about reads like `foo.value;` that are expression statements
353+
// or returns
354+
const parentStmt = node.parent;
266355
if (
267-
node.parent?.type !== AST_NODE_TYPES.ExpressionStatement ||
268-
node.computed ||
269-
node.property.type !== AST_NODE_TYPES.Identifier ||
270-
node.property.name !== 'value'
356+
!parentStmt ||
357+
(parentStmt.type !== AST_NODE_TYPES.ExpressionStatement &&
358+
parentStmt.type !== AST_NODE_TYPES.ReturnStatement)
271359
) {
272360
return;
273361
}
274-
const exprStmt = node.parent as TSESTree.ExpressionStatement;
362+
275363
// Find the top-of-body allowed zone
276364
const body =
277365
currentFn.body && currentFn.body.type === AST_NODE_TYPES.BlockStatement
278366
? currentFn.body
279367
: null;
280368
if (!body) {
369+
// Arrow function with implicit return, e.g. () => mySignal.value
370+
if (currentFn.body.type === AST_NODE_TYPES.MemberExpression && currentFn.body === node) {
371+
// This is ok, it's at the "top"
372+
return;
373+
}
281374
return;
282375
}
283-
// Only consider top-level statements of the QRL callback body
284-
if (exprStmt.parent !== body) {
376+
// Only consider top-level statements of the function body
377+
if (parentStmt.parent !== body) {
285378
return;
286379
}
287380

288-
const allowedFirst = getFirstStatementIfValueRead(body);
289-
const isAtTop = allowedFirst === exprStmt;
290-
if (isAtTop) {
291-
return;
381+
// Check if this .value access is at the top of the function body
382+
// The first statement is allowed (this is the "top")
383+
const firstStmt = body.body[0];
384+
if (firstStmt === parentStmt) {
385+
return; // This is the first statement, OK
292386
}
293387

294-
// Determine if the object is an async computed signal
295-
const obj = node.object;
296-
if (obj.type !== AST_NODE_TYPES.Identifier) {
297-
return;
298-
}
299-
if (!isAsyncComputedIdentifier(context, obj)) {
300-
return;
388+
// Also allow if the first statement is a .value access (even if current is not first)
389+
// This handles the case where the first .value primes the signal
390+
const allowedFirst = getFirstStatementIfValueRead(body);
391+
if (allowedFirst === parentStmt) {
392+
return; // This is the designated first .value access, OK
301393
}
302394

303-
// Allow if there is an earlier 'await <ident>.resolve()' in the same body
304-
if (hasAwaitResolveBefore(body, exprStmt, obj.name)) {
395+
// Allow if there is an earlier 'await <ident>.promise' in the same body.
396+
if (
397+
(parentStmt.type === AST_NODE_TYPES.ExpressionStatement ||
398+
parentStmt.type === AST_NODE_TYPES.ReturnStatement) &&
399+
hasAwaitPromiseBefore(body, parentStmt, obj.name)
400+
) {
305401
return;
306402
}
307403

308404
context.report({
309-
node: node as any,
405+
node,
310406
messageId: 'asyncComputedNotTop',
311407
data: { name: obj.name },
312408
});
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { component$ } from '@qwik.dev/core';
2+
import { routeLoader$, type LoaderSignal } from '@qwik.dev/router';
3+
4+
export const useProductDetails = routeLoader$(async (requestEvent) => {
5+
return 'abc';
6+
});
7+
8+
export default component$(() => {
9+
const signal = useProductDetails();
10+
const x = 0;
11+
// Expect error: { "messageId": "asyncComputedNotTop" }
12+
signal.value;
13+
return <p>Product name: {signal.value}</p>;
14+
});
15+
16+
const useCustom = async (signal: LoaderSignal<string>) => {
17+
const x = 0;
18+
// Expect error: { "messageId": "asyncComputedNotTop" }
19+
return signal.value;
20+
};
21+
22+
export const Test3 = component$(() => {
23+
const signal = useProductDetails();
24+
const x = useCustom(signal);
25+
return <p>Product name: {x}</p>;
26+
});
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { component$ } from '@qwik.dev/core';
2+
import { routeLoader$, type LoaderSignal } from '@qwik.dev/router';
3+
4+
export const useProductDetails = routeLoader$(async (requestEvent) => {
5+
return 'abc';
6+
});
7+
8+
export default component$(() => {
9+
const signal = useProductDetails();
10+
return <p>Product name: {signal.value}</p>;
11+
});
12+
13+
export const useCustom = (signal: any) => {
14+
signal.value;
15+
const x = 0;
16+
};
17+
18+
export const Test2 = component$(() => {
19+
const signal = useProductDetails();
20+
useCustom(signal);
21+
return <p>Product name: {signal.value}</p>;
22+
});
23+
24+
const useCustom2 = async (signal: LoaderSignal<string>) => {
25+
await signal.promise();
26+
const x = 0;
27+
return signal.value;
28+
};
29+
30+
export const Test3 = component$(() => {
31+
const signal = useProductDetails();
32+
const x = useCustom2(signal);
33+
return <p>Product name: {x}</p>;
34+
});

0 commit comments

Comments
 (0)