feat(no-magic-timeouts): flag inline numeric timeout values

[unlikelyzero]

Closes #487

Adds an opt-in no-magic-timeouts rule that reports numeric timeout values passed inline to a Playwright call.

The problem

Playwright's auto-waiting means an explicit timeout is an escape hatch, and in practice those escape hatches accumulate as bare numbers scattered across spec files:

await page.goto('/reports', { timeout: 60000 })
await page.getByRole('button', { name: 'Generate' }).click({ timeout: 5000 })
await expect(page.getByText('Ready')).toBeVisible({ timeout: 30 * 1000 })
test.setTimeout(120000)

None of those numbers records a reason. A reader can't tell whether 5000 was measured, guessed, or bumped in a hurry to get CI green — so nobody dares lower it, and the only safe edit is to raise it. Timeouts in a suite therefore only ratchet upward, with two costs:

1. Changes don't compose. Each literal lives at its own call site, so "make the suite less flaky" becomes a hunt through every spec file. Identical values drift apart and the suite ends up with no coherent timing policy.
2. Regressions get masked. The suite takes its effective upper bound from whichever number is largest. A regression that used to fail in two seconds fails in sixty instead, or passes slowly and goes unnoticed.

Naming the value fixes both — { timeout: REPORT_GENERATION_TIMEOUT } says why the wait exists, and editing the constant edits every site sharing that reason. Better still, most of these belong in playwright.config.ts, where they apply suite-wide.

ESLint core's no-magic-numbers can't do this job: it exempts object properties by default, and enabling detectObjects flags every numeric property in the file. The Playwright API tells us exactly which positions are timeouts, so this rule can be precise where a general-purpose one can't.

What it reports

• any timeout property in an options object passed directly to a call — click(), goto(), waitForURL(), toBeVisible(), toPass(), the test() options object, test.describe.configure()
• test.setTimeout(120000) and testInfo.setTimeout(120000)
• arithmetic built purely from literals, e.g. 30 * 1000 and 2 * 60 * 1000, which is how these are usually written

What it leaves alone

Anything that already has a name or a home:

• a binding or member expression — { timeout: REPORT_TIMEOUT }, { timeout: TIMEOUTS.short }
• a named options object — const uploadOptions = { timeout: 120_000 }
• anything inside defineConfig(), and any object that isn't a call argument

Options

• allow: number[] (default []) — values permitted inline. I'd initially floated defaulting this to [0], since Playwright treats 0 as "disable this timeout" and that is self-documenting. I've kept it empty, and the docs now explain why: no-action-timeout singles out { timeout: 0 } as the worst case, because it removes the bound entirely and lets a broken test hang until the run is killed. Defaulting to [0] would mean one rule blessing what another condemns.
• minOccurrences: number (default 1) — how many times a value must appear in a file before it is reported. See below.
• properties: string[] (default ["timeout"]) — property names treated as timeouts, so projects setting actionTimeout / navigationTimeout via test.use() can cover those.

Implementation

A Property visitor keyed on the property name, gated on the containing ObjectExpression being a direct call argument and not inside defineConfig(), plus a CallExpression visitor for setTimeout. Values are statically evaluated across + - * / and unary +/-; evaluation bails the moment it reaches an identifier, since a named reference is exactly what the rule wants.

Reports are collected during the walk and emitted at Program:exit, grouped by evaluated value, so minOccurrences can be applied once the whole file has been seen — which also means 15_000 and 15 * 1000 count as the same timeout.

Not part of the recommended config — whether a timeout deserves a name is a project-level judgement, and suites that already centralise their timeouts have nothing to fix.

Adoption on an existing suite

Testing this against a mature suite surfaced the real problem with shipping it as originally written: turned on at full strength it reports every inline timeout at once, potentially hundreds, and allow doesn't help because the problem values are all different. A suite in that position simply won't adopt the rule.

It also sharpened what the actual harm is. In that suite, 15_000 appeared at four sites for two different reasons and 45_000 twice for one. The damage is duplication and drift, not literalness — a first, single, local timeout is often defensible; the fifth copy is what ratchets.

minOccurrences targets that directly:

{ "playwright/no-magic-timeouts": ["warn", { "minOccurrences": 2 }] }

With minOccurrences: 2, a value is reported only once it appears twice in the same file, and then every site is flagged.

The honest limitation, which the docs state plainly: ESLint sees one file at a time, so this counts repetitions within a file. It catches the page object that waits 15_000 three times; it won't catch the same value spread across four different page objects. For that, the docs describe the complementary path — enable at error for new and changed files via lint-staged or a scoped overrides block, and widen the glob as directories get cleaned.

One scoping note, since it came up: the rule is not gated to spec files. It hooks Property and CallExpression with no test-context check, so page objects and helper modules — where a mature suite concentrates its waits — are covered. There's now a fixture asserting that.

Checks

yarn test (3372 tests), oxfmt --check ., eslint ., and tsc --noEmit all pass.

[mskelton]

Conflicts here

[github-actions]

🎉 This PR is included in version 2.12.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀