Check if page methods are missing await (missing-playwright-await)
#310
Conversation
| @@ -247,6 +247,21 @@ runRuleTester('missing-playwright-await', rule, { | |||
| }, | |||
| }, | |||
| }, | |||
| // Page methods | |||
There was a problem hiding this comment.
Would love to hear more ideas on test cases here. It's a bit simpler to check page methods than the test/except cases, since it's not a chained call.
There was a problem hiding this comment.
Let's add test cases to assert myPage.goto, frame.goto, myFrame.goto, etc.
| @@ -57,6 +57,76 @@ const playwrightTestMatchers = [ | |||
| 'toBeInViewport', | |||
| ] | |||
|
|
|||
| const pageMethods = new Set([ | |||
There was a problem hiding this comment.
Please double check these 🙏 I tried to alphabetically sort them and make sure I included all methods that return Promise and not any that don't, but I could have made a mistake here.
| @@ -116,10 +116,17 @@ export function dig(node: ESTree.Node, identifier: string | RegExp): boolean { | |||
| : false | |||
| } | |||
|
|
|||
| export function isPage(node: ESTree.CallExpression) { | |||
There was a problem hiding this comment.
I'm kind of dubious on whether this function is a good idea or not, since you have to check if node.callee is a MemberExpression anyway to get the type checking to work properly. Open to thoughts.
There was a problem hiding this comment.
How about instead accepting a MemberExpression and then doing isPage(node.callee).
| { code: `await page.goto('https://example.com')` }, | ||
| { code: `await page.title()` }, | ||
| // Other page methods are ignored | ||
| { code: `page.frames()` }, | ||
| // Other methods with the same name are ignored | ||
| { code: `randomObject.title()` }, | ||
| // Does not need to be awaited when returned | ||
| { code: `() => { return page.content() }` }, |
There was a problem hiding this comment.
Let's add a few tests for this, e.g. this.page.goto()
There was a problem hiding this comment.
Also, this probably doesn't support stuff like this: page.locator('.foo').click(). Without that, this rule is pretty limited in usefulness.
| @@ -247,6 +247,21 @@ runRuleTester('missing-playwright-await', rule, { | |||
| }, | |||
| }, | |||
| }, | |||
| // Page methods | |||
There was a problem hiding this comment.
Let's add test cases to assert myPage.goto, frame.goto, myFrame.goto, etc.
| @@ -116,10 +116,17 @@ export function dig(node: ESTree.Node, identifier: string | RegExp): boolean { | |||
| : false | |||
| } | |||
|
|
|||
| export function isPage(node: ESTree.CallExpression) { | |||
There was a problem hiding this comment.
How about instead accepting a MemberExpression and then doing isPage(node.callee).
| messageId: 'page', | ||
| node: node.callee, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Go ahead and return at the end of this if statement, if it is a page method, no need to run parseFnCall since it won't be a expect or test function
missing-playwright-awaitrule #159This adds support for checking if
Pagemethods are properly awaited, as part of themissing-playwright-awaitrule.I've intentionally skipped the
waitForEvent,waitForRequest, andwaitForResponsemethods, as I believe these should be covered by a different lint rule or PR, since it is more complex: #199