Skip to content

Commit 41297dc

Browse files
MSroczynski3mskelton
MSroczynski3
and
mskelton
authored
Add no-wait-for-navigation rule (#375)
* Added no-wait-for-navigation rule * Format * Examples * Fix lint --------- Co-authored-by: Mark Skelton <mdskelton99@gmail.com>
1 parent 663e2d7 commit 41297dc

File tree

6 files changed

+357
-0
lines changed

6 files changed

+357
-0
lines changed

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ When adding new rules, make sure to follow these steps:
3131
1. Add the rule source code in `src/rules`
3232
1. Add tests for the rule in `src/rules`
3333
1. Add docs for the rule to `docs/rules`
34+
1. Add the rule to the `index.ts`
3435
1. Add a short description of the rule in `README.md`
3536

3637
## Releasing

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ CLI option\
144144
| [no-unsafe-references](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-unsafe-references.md) | Prevent unsafe variable references in `page.evaluate()` | ✅ | 🔧 | |
145145
| [no-useless-await](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-await.md) | Disallow unnecessary `await`s for Playwright methods | ✅ | 🔧 | |
146146
| [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md) | Disallow usage of `not` matchers when a specific matcher exists | ✅ | 🔧 | |
147+
| [no-wait-for-navigation](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-navigation.md) | Disallow usage of `page.waitForNavigation()` | ✅ | | 💡 |
147148
| [no-wait-for-selector](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-selector.md) | Disallow usage of `page.waitForSelector()` | ✅ | | 💡 |
148149
| [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md) | Disallow usage of `page.waitForTimeout()` | ✅ | | 💡 |
149150
| [prefer-comparison-matcher](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-comparison-matcher.md) | Suggest using the built-in comparison matchers | | 🔧 | |

‎docs/rules/no-wait-for-navigation.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Disallow usage of `page.waitForNavigation` (`no-wait-for-navigation`)
2+
3+
## Rule Details
4+
5+
Example of **incorrect** code for this rule:
6+
7+
```javascript
8+
await page.waitForNavigation()
9+
10+
const navigationPromise = page.waitForNavigation()
11+
await page.getByText('Navigate after timeout').click()
12+
await navigationPromise
13+
```
14+
15+
Examples of **correct** code for this rule:
16+
17+
```javascript
18+
await page.waitForURL('**/target')
19+
await page.click('delayed-navigation')
20+
```

‎src/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import noStandaloneExpect from './rules/no-standalone-expect.js'
2525
import noUnsafeReferences from './rules/no-unsafe-references.js'
2626
import noUselessAwait from './rules/no-useless-await.js'
2727
import noUselessNot from './rules/no-useless-not.js'
28+
import noWaitForNavigation from './rules/no-wait-for-navigation.js'
2829
import noWaitForSelector from './rules/no-wait-for-selector.js'
2930
import noWaitForTimeout from './rules/no-wait-for-timeout.js'
3031
import preferComparisonMatcher from './rules/prefer-comparison-matcher.js'
@@ -79,6 +80,7 @@ const index = {
7980
'no-unsafe-references': noUnsafeReferences,
8081
'no-useless-await': noUselessAwait,
8182
'no-useless-not': noUselessNot,
83+
'no-wait-for-navigation': noWaitForNavigation,
8284
'no-wait-for-selector': noWaitForSelector,
8385
'no-wait-for-timeout': noWaitForTimeout,
8486
'prefer-comparison-matcher': preferComparisonMatcher,
@@ -126,6 +128,7 @@ const sharedConfig = {
126128
'playwright/no-unsafe-references': 'error',
127129
'playwright/no-useless-await': 'warn',
128130
'playwright/no-useless-not': 'warn',
131+
'playwright/no-wait-for-navigation': 'error',
129132
'playwright/no-wait-for-selector': 'warn',
130133
'playwright/no-wait-for-timeout': 'warn',
131134
'playwright/prefer-web-first-assertions': 'error',

‎src/rules/no-wait-for-navigation.test.ts‎

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
import rule from '../../src/rules/no-wait-for-navigation.js'
2+
import { runRuleTester } from '../utils/rule-tester.js'
3+
4+
const messageId = 'noWaitForNavigation'
5+
6+
runRuleTester('no-wait-for-navigation', rule, {
7+
invalid: [
8+
{
9+
code: `const navigationPromise = page.waitForNavigation();`,
10+
errors: [
11+
{
12+
column: 27,
13+
line: 1,
14+
messageId,
15+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
16+
},
17+
],
18+
},
19+
{
20+
code: 'async function fn() { await page.waitForNavigation() }',
21+
errors: [
22+
{
23+
column: 29,
24+
endColumn: 53,
25+
endLine: 1,
26+
line: 1,
27+
messageId,
28+
suggestions: [
29+
{
30+
messageId: 'removeWaitForNavigation',
31+
output: 'async function fn() { }',
32+
},
33+
],
34+
},
35+
],
36+
},
37+
{
38+
code: 'async function fn() { await this.page.waitForNavigation() }',
39+
errors: [
40+
{
41+
column: 29,
42+
endColumn: 58,
43+
endLine: 1,
44+
line: 1,
45+
messageId,
46+
suggestions: [
47+
{
48+
messageId: 'removeWaitForNavigation',
49+
output: 'async function fn() { }',
50+
},
51+
],
52+
},
53+
],
54+
},
55+
{
56+
code: 'async function fn() { await page["waitForNavigation"]() }',
57+
errors: [
58+
{
59+
column: 29,
60+
endColumn: 56,
61+
line: 1,
62+
messageId,
63+
suggestions: [
64+
{
65+
messageId: 'removeWaitForNavigation',
66+
output: 'async function fn() { }',
67+
},
68+
],
69+
},
70+
],
71+
},
72+
{
73+
code: 'async function fn() { await page[`waitForNavigation`]() }',
74+
errors: [
75+
{
76+
column: 29,
77+
endColumn: 56,
78+
line: 1,
79+
messageId,
80+
suggestions: [
81+
{
82+
messageId: 'removeWaitForNavigation',
83+
output: 'async function fn() { }',
84+
},
85+
],
86+
},
87+
],
88+
},
89+
{
90+
code: 'async function fn() { return page.waitForNavigation(); }',
91+
errors: [
92+
{
93+
column: 30,
94+
endColumn: 54,
95+
line: 1,
96+
messageId,
97+
suggestions: [
98+
{
99+
messageId: 'removeWaitForNavigation',
100+
output: 'async function fn() { }',
101+
},
102+
],
103+
},
104+
],
105+
},
106+
{
107+
code: 'async function fn() { page.waitForNavigation(); }',
108+
errors: [
109+
{
110+
column: 23,
111+
endColumn: 47,
112+
line: 1,
113+
messageId,
114+
suggestions: [
115+
{
116+
messageId: 'removeWaitForNavigation',
117+
output: 'async function fn() { }',
118+
},
119+
],
120+
},
121+
],
122+
},
123+
{
124+
code: '(async function() { await page.waitForNavigation(); })();',
125+
errors: [
126+
{
127+
column: 27,
128+
endColumn: 51,
129+
line: 1,
130+
messageId,
131+
suggestions: [
132+
{
133+
messageId: 'removeWaitForNavigation',
134+
output: '(async function() { })();',
135+
},
136+
],
137+
},
138+
],
139+
},
140+
{
141+
code: 'page.waitForNavigation()',
142+
errors: [
143+
{
144+
column: 1,
145+
endColumn: 25,
146+
line: 1,
147+
messageId,
148+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
149+
},
150+
],
151+
},
152+
{
153+
code: 'page["waitForNavigation"]()',
154+
errors: [
155+
{
156+
column: 1,
157+
endColumn: 28,
158+
line: 1,
159+
messageId,
160+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
161+
},
162+
],
163+
},
164+
{
165+
code: 'page[`waitForNavigation`]()',
166+
errors: [
167+
{
168+
column: 1,
169+
endColumn: 28,
170+
line: 1,
171+
messageId,
172+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
173+
},
174+
],
175+
},
176+
{
177+
code: 'foo.page().waitForNavigation()',
178+
errors: [
179+
{
180+
column: 1,
181+
endColumn: 31,
182+
line: 1,
183+
messageId,
184+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
185+
},
186+
],
187+
},
188+
{
189+
code: 'this.foo().page().waitForNavigation()',
190+
errors: [
191+
{
192+
column: 1,
193+
endColumn: 38,
194+
line: 1,
195+
messageId,
196+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
197+
},
198+
],
199+
},
200+
{
201+
code: 'page2.waitForNavigation()',
202+
errors: [
203+
{
204+
column: 1,
205+
endColumn: 26,
206+
line: 1,
207+
messageId,
208+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
209+
},
210+
],
211+
},
212+
{
213+
code: 'this.page2.waitForNavigation()',
214+
errors: [
215+
{
216+
column: 1,
217+
endColumn: 31,
218+
line: 1,
219+
messageId,
220+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
221+
},
222+
],
223+
},
224+
{
225+
code: 'myPage.waitForNavigation()',
226+
errors: [
227+
{
228+
column: 1,
229+
endColumn: 27,
230+
line: 1,
231+
messageId,
232+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
233+
},
234+
],
235+
},
236+
{
237+
code: 'this.myPage.waitForNavigation()',
238+
errors: [
239+
{
240+
column: 1,
241+
endColumn: 32,
242+
line: 1,
243+
messageId,
244+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
245+
},
246+
],
247+
},
248+
{
249+
code: 'page.waitForNavigation({ waitUntil: "load" })',
250+
errors: [
251+
{
252+
column: 1,
253+
endColumn: 46,
254+
line: 1,
255+
messageId,
256+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
257+
},
258+
],
259+
},
260+
{
261+
code: 'page.waitForNavigation({ waitUntil: "domcontentloaded" })',
262+
errors: [
263+
{
264+
column: 1,
265+
endColumn: 58,
266+
line: 1,
267+
messageId,
268+
suggestions: [{ messageId: 'removeWaitForNavigation', output: '' }],
269+
},
270+
],
271+
},
272+
],
273+
valid: [
274+
'function waitForNavigation() {}',
275+
'async function fn() { await waitForNavigation(); }',
276+
'async function fn() { await this.foo.waitForNavigation(); }',
277+
'(async function() { await page.waitForSelector("#foo"); })();',
278+
'page.waitForSelector("#foo");',
279+
'page["waitForSelector"]("#foo");',
280+
'page.waitForTimeout(2000);',
281+
'page["waitForTimeout"](2000);',
282+
'rampage.waitForNavigation();',
283+
'myPage2.waitForNavigation();',
284+
'page.waitForLoadState("load");',
285+
'page.waitForLoadState("domcontentloaded");',
286+
'page.waitForLoadState("networkidle");',
287+
],
288+
})

‎src/rules/no-wait-for-navigation.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { isPageMethod } from '../utils/ast.js'
2+
import { createRule } from '../utils/createRule.js'
3+
4+
export default createRule({
5+
create(context) {
6+
return {
7+
CallExpression(node) {
8+
if (isPageMethod(node, 'waitForNavigation')) {
9+
context.report({
10+
messageId: 'noWaitForNavigation',
11+
node,
12+
suggest: [
13+
{
14+
fix: (fixer) =>
15+
fixer.remove(
16+
node.parent &&
17+
node.parent.type !== 'AwaitExpression' &&
18+
node.parent.type !== 'VariableDeclarator'
19+
? node.parent
20+
: node.parent.parent,
21+
),
22+
messageId: 'removeWaitForNavigation',
23+
},
24+
],
25+
})
26+
}
27+
},
28+
}
29+
},
30+
meta: {
31+
docs: {
32+
category: 'Possible Errors',
33+
description: 'Prevent usage of page.waitForNavigation()',
34+
recommended: true,
35+
url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-navigation.md',
36+
},
37+
hasSuggestions: true,
38+
messages: {
39+
noWaitForNavigation: 'Unexpected use of page.waitForNavigation().',
40+
removeWaitForNavigation: 'Remove the page.waitForNavigation() method.',
41+
},
42+
type: 'suggestion',
43+
},
44+
})

0 commit comments

Comments
 (0)