Skip to content

Commit d26d6e4

Browse files
authored
Add new "noWaitForTimeout" rule (playwright-community#46)
* rule: add new noElementHandle rule * chore: move end range to const * fix: getRange method for non-awaiting statements * test: add additional tests for the noElementHandle rule * chore: improve noElementHandle rule * docs: update the wording of noElementHandle rule * rule: update message to match with other rule naming conventions * rule: change noElementHandle to warn level * rule: noElementHandle change fixable to suggestion * test: add valid tests for noElementHandle rule * rule: change type for noElementHandle to suggestion * rule: update noElementHandle rule's message for suggestions * rule: update noElementHandle with getRange method * feature: add noWaitForTimeout rule
1 parent 13afef3 commit d26d6e4

File tree

4 files changed

+99
-0
lines changed

4 files changed

+99
-0
lines changed

README.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,3 +190,24 @@ test.describe.serial('two tests in serial mode', () => {
190190
test('two', async ({ page }) => {});
191191
});
192192
```
193+
194+
### `no-wait-for-timeout`
195+
196+
Disallow usage of `page.waitForTimeout()`.
197+
198+
Example of **incorrect** code for this rule:
199+
200+
```js
201+
await page.waitForTimeout(5000);
202+
```
203+
204+
Examples of **correct** code for this rule:
205+
206+
```js
207+
// Use signals such as network events, selectors becoming visible and others instead.
208+
await page.waitForLoadState();
209+
210+
await page.waitForUrl('/home');
211+
212+
await page.waitForFunction(() => window.innerWidth < 100);
213+
```

lib/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const noPagePause = require("./rules/no-page-pause");
33
const noElementHandle = require("./rules/no-element-handle");
44
const noEval = require("./rules/no-eval");
55
const noFocusedTest = require("./rules/no-focused-test");
6+
const noWaitForTimeout = require("./rules/no-wait-for-timeout");
67

78
module.exports = {
89
configs: {
@@ -18,6 +19,7 @@ module.exports = {
1819
"playwright/no-element-handle": "warn",
1920
"playwright/no-eval": "warn",
2021
"playwright/no-focused-test": "error",
22+
"playwright/no-wait-for-timeout": "warn",
2123
},
2224
},
2325
"jest-playwright": {
@@ -59,5 +61,6 @@ module.exports = {
5961
"no-element-handle": noElementHandle,
6062
"no-eval": noEval,
6163
"no-focused-test": noFocusedTest,
64+
"no-wait-for-timeout": noWaitForTimeout,
6265
},
6366
};

lib/rules/no-wait-for-timeout.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
const { isPageIdentifier, isCalleeProperty } = require('../utils/ast');
2+
3+
/** @type {import('eslint').Rule.RuleModule} */
4+
module.exports = {
5+
create(context) {
6+
return {
7+
CallExpression(node) {
8+
if (isPageIdentifier(node) && isCalleeProperty(node, 'waitForTimeout')) {
9+
context.report({
10+
messageId: 'noWaitForTimeout',
11+
suggest: [
12+
{
13+
messageId: 'removeWaitForTimeout',
14+
fix: (fixer) =>
15+
fixer.remove(
16+
node.parent && node.parent.type !== 'AwaitExpression' ? node.parent : node.parent.parent
17+
),
18+
},
19+
],
20+
node,
21+
});
22+
}
23+
},
24+
};
25+
},
26+
meta: {
27+
docs: {
28+
category: 'Best Practices',
29+
description: 'Prevent usage of page.waitForTimeout()',
30+
recommended: true,
31+
url: 'https://github.com/playwright-community/eslint-plugin-playwright#no-wait-for-timeout',
32+
},
33+
hasSuggestions: true,
34+
messages: {
35+
noWaitForTimeout: 'Unexpected use of page.waitForTimeout().',
36+
removeWaitForTimeout: 'Remove the page.waitForTimeout() method.',
37+
},
38+
type: 'suggestion',
39+
},
40+
};

test/no-wait-for-timeout.spec.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
const { RuleTester } = require('eslint');
2+
const rule = require('../lib/rules/no-wait-for-timeout');
3+
4+
RuleTester.setDefaultConfig({
5+
parserOptions: {
6+
ecmaVersion: 2018,
7+
},
8+
});
9+
10+
const invalid = (code, output) => ({
11+
code,
12+
errors: [
13+
{
14+
messageId: 'noWaitForTimeout',
15+
suggestions: [{ messageId: 'removeWaitForTimeout', output }],
16+
},
17+
],
18+
});
19+
20+
new RuleTester().run('no-wait-for-timeout', rule, {
21+
invalid: [
22+
invalid(`async function fn() { await page.waitForTimeout(1000) }`, 'async function fn() { }'),
23+
invalid('async function fn() { return page.waitForTimeout(1000); }', 'async function fn() { }'),
24+
invalid('async function fn() { page.waitForTimeout(1000); }', 'async function fn() { }'),
25+
invalid('(async function() { await page.waitForTimeout(500); })();', '(async function() { })();'),
26+
invalid('page.waitForTimeout(1000);', ''),
27+
invalid('page.waitForTimeout(2000)', ''),
28+
],
29+
valid: [
30+
'function waitForTimeout() {}',
31+
'async function fn() { await waitForTimeout(4343); }',
32+
'(async function() { await page.waitForSelector("#foo"); })();',
33+
'page.waitForSelector("#foo");',
34+
],
35+
});

0 commit comments

Comments
 (0)