KittyCAD / modeling-app

The KittyCAD modeling app.
https://kittycad.io/modeling-app/download
MIT License
371 stars 31 forks source link

Add and ESLint rule to not allow use of Playwright's `page.click()` method #2624

Open franknoirot opened 4 months ago

franknoirot commented 4 months ago

It has caused errant clicks for us in our test suite because we also use page.mouse.move() and page.mouse.click() to click in specific places in the stream. If you have moved the mouse to a location and then call page.click(), even if the selector isn't under the mouse's position, two clicks will register simultaneously with Playwright: one where the mouse was left, one on the target of page.click(). Playwright's docs say that page.click() is "discouraged" in favor of page.locator().click(), so we should have an ESLint rule for it.

franknoirot commented 4 months ago

Additional note: this is not actually the behavior that was leading to the duplicate clicks; making this replacement still resulted in the same behavior. The fix for that issue was to call page.mouse.move(0,0) before the next call to any kind of click. So this is more of a best practice following guardrail than fixing the main issue we run into with Playwright clicks.

Irev-Dev commented 5 days ago

If we're adding playwright eslint rules, I think we should error for page.waitForTimeout

It would a pain to add because it would have to be disable in currently working tests, of which there are 370ish uses