Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
b27c7fd
feat(lint): add Playwright ESLint rules
abossenbroek Feb 4, 2026
003caa6
fix(lint): fix markup! interpolation in noPlaywrightEval
abossenbroek Feb 4, 2026
bbf011d
fix(lint): add 'clear' method to noPlaywrightForceOption
abossenbroek Feb 4, 2026
ddc5641
fix(lint): handle parenthesized await in noPlaywrightMissingAwait
abossenbroek Feb 4, 2026
a982006
docs(lint): fix example conflict in noPlaywrightPagePause
abossenbroek Feb 4, 2026
51a78b6
docs: fix changeset wording for noPlaywrightElementHandle
abossenbroek Feb 4, 2026
6c10cbf
fix(test): allow large_stack_arrays in generated test code
abossenbroek Feb 4, 2026
00cfe4e
feat(lint): add auto-fix to noPlaywrightSkippedTest
abossenbroek Feb 4, 2026
cefb809
feat(lint): add auto-fix to noPlaywrightElementHandle
abossenbroek Feb 4, 2026
8d04317
feat(lint): add auto-fix to noPlaywrightWaitForSelector
abossenbroek Feb 4, 2026
f7835ba
feat(lint): add web-first assertions note to noPlaywrightWaitForTimeout
abossenbroek Feb 4, 2026
8f551b3
feat(lint): add expectPlaywrightExpect rule
abossenbroek Feb 4, 2026
64eac4b
feat(lint): add noPlaywrightConditionalExpect rule
abossenbroek Feb 4, 2026
18ed78a
chore: apply clippy fixes
abossenbroek Feb 4, 2026
103b9b0
chore: regenerate analyzer code
abossenbroek Feb 4, 2026
c4d4aeb
chore: regenerate JS bindings
abossenbroek Feb 4, 2026
fc7fd4b
chore: address remaining CodeRabbit review nitpicks
abossenbroek Feb 4, 2026
2863f25
chore: address PR #8960 review comments for Playwright rules
abossenbroek Feb 4, 2026
9268439
test(noPlaywrightForceOption): add parenthesized objects test
abossenbroek Feb 4, 2026
aa7874e
test(noPlaywrightUselessAwait): add .not modifier test
abossenbroek Feb 4, 2026
0a2063c
test(noPlaywrightMissingAwait): add non-expect poll valid test
abossenbroek Feb 4, 2026
39ff810
test(usePlaywrightExpect): add options callback test
abossenbroek Feb 4, 2026
a4260b7
refactor(playwright): move is_expect_call to shared playwright.rs
abossenbroek Feb 4, 2026
ce230d3
test(noPlaywrightConditionalExpect): add expect modifier tests
abossenbroek Feb 4, 2026
09189cc
refactor(playwright): improve is_expect_call and TokenText comparisons
abossenbroek Feb 4, 2026
d37883c
fix(playwright): exclude describe blocks, hooks, and steps from is_te…
abossenbroek Feb 4, 2026
1456cc0
refactor(playwright): move contains_expect_call to shared playwright.rs
abossenbroek Feb 4, 2026
bbfcde2
test(playwright): add explicit expect.poll().not valid test case
abossenbroek Feb 5, 2026
18dea40
[autofix.ci] apply automated fixes
autofix-ci[bot] Feb 5, 2026
2c69c23
test(playwright): add unit tests for is_test_call function
abossenbroek Feb 5, 2026
150f677
[autofix.ci] apply automated fixes
autofix-ci[bot] Feb 5, 2026
bee5b39
refactor(playwright): address PR #8960 review feedback from @dyc3
abossenbroek Feb 7, 2026
9906d42
docs(playwright): wrap inline JS example in fenced code block
abossenbroek Feb 7, 2026
e05f671
fix(playwright): address round 4 review feedback on PR #8960
abossenbroek Feb 7, 2026
5e04d85
refactor(playwright): extract shared utilities and reduce code duplic…
abossenbroek Feb 7, 2026
5e8361a
fix(playwright): address round 5 CodeRabbit review feedback on PR #8960
abossenbroek Feb 7, 2026
c215b23
refactor(playwright): consolidate noPlaywrightSkippedTest into noSkip…
abossenbroek Feb 7, 2026
f85fe12
fix(playwright): address round 6 review feedback on PR #8960
abossenbroek Feb 9, 2026
91143ae
fix(playwright): address round 7 CodeRabbit nitpick comments on PR #8960
abossenbroek Feb 9, 2026
ca0bb78
fix(playwright): address round 8 CodeRabbit review comments on PR #8960
abossenbroek Feb 9, 2026
2ca5ad7
fix(playwright): resolve CI failures after rebase on main
abossenbroek Feb 11, 2026
971273e
fix(playwright): resolve clippy lints for collapsible if, needless bo…
abossenbroek Feb 12, 2026
20cc956
Merge branch 'main' into feat/playwright-eslint-rules
abossenbroek Feb 13, 2026
1afeeef
fix(playwright): resolve merge conflicts with main
abossenbroek Feb 16, 2026
650a954
Merge branch 'feat/playwright-eslint-rules' of github.com:abossenbroe…
abossenbroek Feb 16, 2026
d4b40dd
fix(noPlaywrightMissingAwait): fix false positives with jest-dom matc…
abossenbroek Feb 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-no-playwright-missing-await-false-positives.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": patch
---

Fixed [#9115](https://github.com/biomejs/biome/issues/9115): The `noPlaywrightMissingAwait` rule no longer produces false positives on jest-dom matchers like `toBeVisible`, `toBeChecked`, `toHaveAttribute`, etc. For matchers shared between Playwright and jest-dom, the rule now checks whether `expect()`'s argument is a Playwright locator or page object before flagging.
19 changes: 19 additions & 0 deletions .changeset/no-conditional-expect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noConditionalExpect`](https://biomejs.dev/linter/rules/no-conditional-expect/). This rule disallows conditional `expect()` calls inside tests, which can lead to tests that silently pass when assertions never run.

```js
// Invalid - conditional expect may not run
test("conditional", async ({ page }) => {
if (someCondition) {
await expect(page).toHaveTitle("Title");
}
});

// Valid - unconditional expect
test("unconditional", async ({ page }) => {
await expect(page).toHaveTitle("Title");
});
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-element-handle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightElementHandle`](https://biomejs.dev/linter/rules/no-playwright-element-handle/). Prefers locators to element handles.

```js
const el = await page.$('.btn');
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-eval.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightEval`](https://biomejs.dev/linter/rules/no-playwright-eval/). Disallows `page.$eval()` and `page.$$eval()` methods.

```js
await page.$eval('.btn', el => el.textContent);
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-force-option.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightForceOption`](https://biomejs.dev/linter/rules/no-playwright-force-option/). Disallows the `force` option on user interactions.

```js
await locator.click({ force: true });
```
10 changes: 10 additions & 0 deletions .changeset/no-playwright-missing-await.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightMissingAwait`](https://biomejs.dev/linter/rules/no-playwright-missing-await/). Enforces awaiting async Playwright APIs.

```js
const el = page.locator('.btn');
el.click(); // Missing await
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-networkidle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightNetworkidle`](https://biomejs.dev/linter/rules/no-playwright-networkidle/). Disallows deprecated `networkidle` wait option.

```js
await page.goto(url, { waitUntil: 'networkidle' });
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-page-pause.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightPagePause`](https://biomejs.dev/linter/rules/no-playwright-page-pause/). Disallows `page.pause()` debugging calls in committed code.

```js
await page.pause();
```
10 changes: 10 additions & 0 deletions .changeset/no-playwright-useless-await.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightUselessAwait`](https://biomejs.dev/linter/rules/no-playwright-useless-await/). Disallows unnecessary `await` on synchronous Playwright methods.

```js
// Incorrect - locator() is synchronous
const loc = await page.locator('.btn');
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-wait-for-navigation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightWaitForNavigation`](https://biomejs.dev/linter/rules/no-playwright-wait-for-navigation/). Prefers modern navigation APIs over deprecated `waitForNavigation()`.

```js
await page.waitForNavigation();
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-wait-for-selector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightWaitForSelector`](https://biomejs.dev/linter/rules/no-playwright-wait-for-selector/). Prefers locators over deprecated `waitForSelector()`.

```js
await page.waitForSelector('.btn');
```
9 changes: 9 additions & 0 deletions .changeset/no-playwright-wait-for-timeout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`noPlaywrightWaitForTimeout`](https://biomejs.dev/linter/rules/no-playwright-wait-for-timeout/). Disallows hard-coded timeouts with `waitForTimeout()`.

```js
await page.waitForTimeout(5000);
```
5 changes: 5 additions & 0 deletions .changeset/no-skipped-tests-enhancement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": patch
---

Enhanced `noSkippedTests` to detect Playwright patterns (`.fixme`, `test.describe`, `test.step`, bracket notation, bare calls). Consolidated `noPlaywrightSkippedTest` into this rule.
17 changes: 17 additions & 0 deletions .changeset/use-expect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`useExpect`](https://biomejs.dev/linter/rules/use-expect/). This rule ensures that test functions contain at least one `expect()` assertion.

```js
// Invalid - test without assertion
test("no assertion", async ({ page }) => {
await page.goto("/");
});

// Valid - test with assertion
test("has assertion", async ({ page }) => {
await expect(page).toHaveTitle("Title");
});
```
9 changes: 9 additions & 0 deletions .changeset/use-playwright-valid-describe-callback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@biomejs/biome": patch
---

Added the nursery rule [`usePlaywrightValidDescribeCallback`](https://biomejs.dev/linter/rules/use-playwright-valid-describe-callback/). Validates that describe callback signatures are not async.

```js
test.describe('suite', async () => {});
```
136 changes: 136 additions & 0 deletions PR_8960_COMMENTS_SUMMARY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# PR #8960 Review Comments Summary

## Overview
- **Total Comments**: 94
- **Files Reviewed**: 29
- **Review Period**: 2026-02-04 to 2026-02-08
- **Status**: Addressed all major feedback

## Comments by Reviewer

| Reviewer | Count | Role |
|----------|-------|------|
| abossenbroek | 42 | Author (self-review & fixes) |
| coderabbitai[bot] | 30 | Automated code quality review |
| dyc3 | 20 | Core Contributor (main technical review) |
| changeset-bot[bot] | 1 | Automated changeset detection |
| codspeed-hq[bot] | 1 | Performance tracking |

## Key Files with Most Comments

### Code Files
1. `crates/biome_js_analyze/src/frameworks/playwright.rs` - 10 comments
2. `crates/biome_js_analyze/src/lint/nursery/no_playwright_skipped_test.rs` - 7 comments
3. `crates/biome_js_analyze/src/lint/nursery/no_playwright_missing_await.rs` - 6 comments
4. `crates/biome_js_analyze/src/lint/nursery/expect_playwright_expect.rs` - 6 comments
5. `crates/biome_js_analyze/src/lint/nursery/use_playwright_valid_describe_callback.rs` - 6 comments

### Changeset Files
- `.changeset/no-playwright-element-handle-fix.md` - 2 comments
- `.changeset/no-playwright-skipped-test-fix.md` - 2 comments
- `.changeset/no-playwright-useless-await.md` - 2 comments
- `.changeset/no-playwright-wait-for-selector-fix.md` - 2 comments
- `.changeset/no-playwright-wait-for-timeout-note.md` - 2 comments

## Major Review Feedback from dyc3

### 1. Naming & Rule Structure Issues
- **Rule Naming**: `expect_playwright_expect` doesn't follow naming guidelines (must start with `use` or `no`)
- **Resolution**: Renamed to follow conventions

### 2. Code Organization
- **Issue**: Duplicate/shared functionality across multiple rule files
- **Suggestion**: Move shared functionality to `crates/biome_js_analyze/src/frameworks/playwright.rs`
- **Affected Files**:
- `expect_playwright_expect.rs`
- `use_playwright_expect.rs`

### 3. Performance Improvements
- **Issue**: String allocations with `.to_string()` in `no_playwright_skipped_test.rs`
- **Suggestion**: Use `inner_string_text()` and `TokenText` instead of `String` to avoid allocations
- **Multiple locations** needed optimization

### 4. Helper Function Parameter Types
- **Issue**: Helper functions using `&TokenText` when they could use `&str`
- **File**: `use_playwright_valid_describe_callback.rs`
- **Rule**: Use `&str` for helper functions, call `.text()` at call sites
- **Note**: CodeRabbit was instructed to add this to its learnings

### 5. Code Structure Standards
- **Issue**: Helper functions/structs/enums placement
- **Rule**: All helpers must go BELOW the `impl Rule` block
- **Exception**: Node unions used in rule queries can stay above for readability
- **File**: `use_playwright_valid_describe_callback.rs`

### 6. Diagnostic Message Ordering
- **Issue**: Diagnostic notes in wrong order
- **File**: `use_playwright_expect.rs` (line 97)
- **Standard Format**:
1. WHAT is the error (main message)
2. WHY is it an error (first note)
3. HOW to fix it (second note)

### 7. Rule Consolidation
- **Issue**: `no_playwright_skipped_test.rs` may be redundant
- **Question**: Does `no_skipped_tests` already cover this functionality?
- **Suggestion**: Consider adding metadata to existing rule instead of new rule

### 8. Rule Domain Placement
- **Issue**: `no_playwright_conditional_expect.rs` is playwright-specific but similar rules exist in other domains
- **Similar Rules**:
- jest: `no-conditional-expect` (jest-community/eslint-plugin-jest)
- vitest: `no-conditional-expect` (vitest-dev/eslint-plugin-vitest)
- **Suggestion**: Move to `test` domain, rename to `noConditionalExpect`, add jest/vitest as rule sources

### 9. Logic Clarity Issues
- **File**: `ast_utils.rs`
- **Issue**: Comment regarding async keyword detection logic unclear
- **Resolution**: Requested clarification/elaboration

### 10. Versioning Constraints
- **File**: `crates/biome_rule_options/src/no_skipped_tests.rs`
- **Issue**: Cannot add new options to rules in patch releases
- **Constraint**: New options must go in separate PR on `next` branch
- **Resolution**: These changes will need to be moved to next branch PR

### 11. Cross-Domain Rule Suggestions
- **File**: `use_playwright_expect.rs`
- **Observation**: Jest and Vitest almost certainly have equivalent rules
- **Action**: Consider adding those as rule sources in documentation

## Changeset Issues Fixed

Multiple changeset files had issues that were corrected:
1. **no-playwright-wait-for-timeout-note.md** - Entire changeset was wrong (fixed)
2. **no-playwright-wait-for-selector-fix.md** - Entire changeset was wrong (fixed)
3. **no-playwright-useless-await.md** - Wording issue, needed "Added the nursery rule..." format (fixed)
4. **no-playwright-skipped-test-fix.md** - Entire changeset was wrong (fixed)
5. **no-playwright-element-handle-fix.md** - Needs to be removed entirely (fixed)
6. **no-playwright-element-handle.md** - Minor wording tweak from "Prefers... over" to "Prefers... to" (fixed)

## CodeRabbit Suggestions

CodeRabbit identified 30 comments primarily focused on:
- Grammar/language improvements in documentation
- Wording consistency in changeset descriptions
- Code style suggestions
- Best practices for PR descriptions

## Action Items Addressed

All major feedback has been addressed:
- ✅ Rule naming conventions fixed
- ✅ Code reorganized for better structure
- ✅ Performance optimizations (string allocation reduction)
- ✅ Helper function signatures corrected
- ✅ Code placement standards followed
- ✅ Diagnostic message ordering fixed
- ✅ Changeset corrections applied
- ✅ Rule consolidation considerations noted for future work
- ⚠️ New options removal needed (will be separate PR on `next` branch)
- 📋 Cross-domain rule sources to be added in documentation

## Files Full Analysis

See `/Users/antonbossenbroek/Documents_local/projects/biome/PR_8960_REVIEW_COMMENTS.md` for the complete detailed transcript of all 94 comments, organized by file and in chronological order.

Loading