playwright-community / eslint-plugin-playwright

ESLint plugin for Playwright
https://www.npmjs.com/package/eslint-plugin-playwright
MIT License
264 stars 36 forks source link

Prefer using native `getBy*` selectors #203

Closed camchenry closed 2 weeks ago

camchenry commented 8 months ago

I've been going through our fairly large suite of Playwright tests and I noticed a common pattern that I think would be great to correct. If I have some time, I will try to contribute a rule for this.

The issue is that developers write a test like expect(page.locator('[aria-label="View more"]')) which works great until the element is updated to use aria-labelledby for labeling, instead of explicit aria-label attributes. From a semantic perspective this is equivalent, but it breaks the Playwright test. We should encourage people to use getByLabel instead in the first place to prevent this from breaking. It is easier to read, easier to maintain, and encourages an accessibility-first focus.

Examples:

// Incorrect:
page.locator('[aria-label="View more"]')
// Correct:
page.getByLabel('View more')
// Incorrect:
page.locator('[role="button"]')
// Correct:
page.getByRole('button')
// Incorrect:
page.locator('[placeholder="Enter some text..."]')
// Correct:
page.getByPlaceholder('Enter some text...')

This could also be an extension to no-raw-locators, but more like a specific auto-fixer that should be safe to use. For example, we don't use no-raw-locators in our codebase because we query plenty of attributes that are fine to use, but a rule like this would be very useful for us.

mskelton commented 7 months ago

This rule seems reasonable enough that we can implement it. There will definitely be edge cases it can't detect as I think keeping the rule fairly simple for common selectors like you have shown makes the most sense. Also once we have the base rule, we can add additional test cases for things we want to catch.