feat(eslint): add a Playwright concern covering specs and their config - #38
Conversation
Playwright defaults actionTimeout and navigationTimeout to 0, which means "no timeout" rather than a sensible one. expect() carries its own default, so a config that sets expect.timeout looks bounded -- but that ceiling only covers assertions. A bare .click() or .goto() on an element that never becomes actionable waits for the whole test timeout. On a suite with a generous per-test budget that is a silent blackout: no output, no failing assertion, and a final error naming the test rather than the call that hung. A real 30-minute per-test budget turned one unactionable click into 24 minutes of silence with the box completely idle, and the eventual message pointed at the test, not the locator. The rule reads the default-exported config -- through defineConfig() and through a satisfies/as wrapper -- and reports each missing timeout. A config with no `use` block at all is reported too, since it inherits both defaults just the same.
Adds eslint-plugin-playwright and wires its recommended set for specs, plus the tier it ships but leaves out of recommended -- matcher choices that make a failure legible, require-to-pass-timeout, locator quality, and structure. All at error: this package is the bar new work is written against, so the standard states the target rather than the current state of any one suite. A repo adopting mid-stream downgrades a rule in its own config, which keeps the exception visible and local. Pure convention stays off: title casing, tag vocabularies, soft assertions, an arbitrary expect budget, and no-hooks (which contradicts require-hook). The config half is a local rule, because the plugin has nothing for it. Playwright defaults actionTimeout and navigationTimeout to 0 -- "no timeout" rather than a sensible one. expect() carries its own default, so a config setting expect.timeout looks bounded, but that ceiling only covers assertions. A bare .click() or .goto() on an element that never becomes actionable waits out the whole test budget. On a 30-minute destructive budget that was 24 minutes of silence with the box completely idle, and the error named the test rather than the locator; with the ceiling in place the same run failed in 3 minutes naming the exact selector. Measured against a real 63-spec suite: 318 findings, no parse failures.
📝 WalkthroughWalkthroughAdds a public ChangesPlaywright ESLint concern
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ESLint
participant PlaywrightConfig as src/eslint/playwright.js
participant PlaywrightPlugin as eslint-plugin-playwright
ESLint->>PlaywrightConfig: Match Playwright spec or config file
PlaywrightConfig->>PlaywrightPlugin: Apply recommended and curated spec rules
PlaywrightPlugin-->>ESLint: Return spec diagnostics
PlaywrightConfig-->>ESLint: Report missing timeout settings
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
test/playwright-spec-rules.test.mjs (1)
67-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the effective severity, not key absence.
The test asserts that the rule key is absent. A plugin release can add the same rule to
flat/recommendedwith severity"off". The key then exists, the rule stays disabled, and this test fails for no real regression. Check the severity instead.♻️ Proposed assertion change
- const enabled = new Set(Object.keys(concern.rules)); + const severityOf = (rule) => { + const entry = concern.rules[rule]; + return Array.isArray(entry) ? entry[0] : entry; + }; for (const rule of [ "playwright/prefer-lowercase-title", "playwright/require-tags", "playwright/require-soft-assertions", "playwright/max-expects", "playwright/no-hooks", ]) { - assert.ok(!enabled.has(rule), `${rule} should stay off`); + const severity = severityOf(rule); + assert.ok( + severity === undefined || severity === "off" || severity === 0, + `${rule} should stay off, got ${severity}`, + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/playwright-spec-rules.test.mjs` around lines 67 - 79, Update the rule loop in the test named “keeps pure-convention rules off, so strictness stays about correctness” to inspect each rule’s configured severity in concern.rules rather than asserting the rule key is absent. Assert that every listed rule is effectively disabled, including when the plugin exposes it with severity "off".src/eslint/playwright.js (1)
86-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIndirect
usevalues produce false reports.
targetbecomesnullwhenuseis not an object literal. Two realistic shapes hit this:
use: sharedUse— a value imported or defined elsewhere.use: { ...sharedUse, baseURL: "..." }— a spread that can carry both timeouts.In both cases the rule reports both timeouts as missing, and the author cannot satisfy it without inlining the values. Consider skipping the report when
useis not an object literal, or when the object literal contains aSpreadElement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/eslint/playwright.js` around lines 86 - 101, Update the target selection and timeout-reporting logic around REQUIRED_TIMEOUTS so indirect use values are not reported as missing: skip validation when use is not an ObjectExpression, and also skip it when that object contains a SpreadElement. Preserve the existing per-property checks and config-object reporting for direct, fully explicit use objects.test/playwright-action-timeouts.test.mjs (1)
71-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the TypeScript wrapper paths.
configObjectinsrc/eslint/playwright.jsunwrapsTSSatisfiesExpressionandTSAsExpression. No test exercises those branches. The default parser used here cannot parse either syntax, so the branches stay unverified. Add a case that setslanguageOptions.parsertotypescript-eslint's parser and lintsexport default defineConfig({...}) satisfies PlaywrightTestConfig;.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/playwright-action-timeouts.test.mjs` around lines 71 - 80, Add a test in test/playwright-action-timeouts.test.mjs that configures languageOptions.parser with the TypeScript ESLint parser and lints an export default expression using defineConfig({...}) satisfies PlaywrightTestConfig. Assert the expected clean result, exercising both TSSatisfiesExpression and TSAsExpression unwrapping in configObject.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/eslint/globs.js`:
- Around line 31-39: Extend the PLAYWRIGHT_SPEC_FILES glob’s extension set to
include mts, cts, and jsx alongside the existing TypeScript, JavaScript, and
module variants, preserving its current spec/test filename matching behavior.
In `@src/eslint/playwright.js`:
- Around line 82-84: Update the rule around ExportDefaultDeclaration to also
inspect CommonJS module.exports assignments matched by PLAYWRIGHT_CONFIG_FILES.
Extract the existing config validation logic into a local checkConfig(config)
helper, then invoke it for both default exports and module.exports values,
including defineConfig({...}) wrappers.
- Around line 156-161: Update the Playwright spec configuration around
PLAYWRIGHT_SPEC_FILES so TypeScript and TSX files are not processed without a
compatible TypeScript parser concern; either document and enforce the required
composition in the README or exclude .ts/.tsx from this entry unless the parser
is supplied through composition. Preserve the existing JavaScript Playwright
rules and recommended configuration.
---
Nitpick comments:
In `@src/eslint/playwright.js`:
- Around line 86-101: Update the target selection and timeout-reporting logic
around REQUIRED_TIMEOUTS so indirect use values are not reported as missing:
skip validation when use is not an ObjectExpression, and also skip it when that
object contains a SpreadElement. Preserve the existing per-property checks and
config-object reporting for direct, fully explicit use objects.
In `@test/playwright-action-timeouts.test.mjs`:
- Around line 71-80: Add a test in test/playwright-action-timeouts.test.mjs that
configures languageOptions.parser with the TypeScript ESLint parser and lints an
export default expression using defineConfig({...}) satisfies
PlaywrightTestConfig. Assert the expected clean result, exercising both
TSSatisfiesExpression and TSAsExpression unwrapping in configObject.
In `@test/playwright-spec-rules.test.mjs`:
- Around line 67-79: Update the rule loop in the test named “keeps
pure-convention rules off, so strictness stays about correctness” to inspect
each rule’s configured severity in concern.rules rather than asserting the rule
key is absent. Assert that every listed rule is effectively disabled, including
when the plugin exposes it with severity "off".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d091f5b4-befd-4ebd-b8e1-951b1e4b2dec
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
README.mdpackage.jsonsrc/eslint/globs.jssrc/eslint/playwright.jstest/playwright-action-timeouts.test.mjstest/playwright-spec-rules.test.mjs
| /** | ||
| * Playwright specs — mirrors Playwright's own default `testMatch`. | ||
| * | ||
| * The plugin's rules key off bare `test`/`expect` identifiers, so in a repo that | ||
| * also runs Vitest or Jest under this pattern, re-scope `files` on the spec entry | ||
| * to wherever Playwright actually owns (`e2e/**`, `**\/*.e2e.spec.ts`, …). | ||
| * Otherwise the rules give Playwright advice about a unit test. | ||
| */ | ||
| export const PLAYWRIGHT_SPEC_FILES = ["**/*.{spec,test}.{ts,tsx,js,mjs,cjs}"]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Playwright default testMatch pattern file extensions
💡 Result:
By default, Playwright uses the following glob pattern to identify test files: /*.@(spec|test).?(c|m)[jt]s?(x) [1][2][3]. This pattern includes the following file types and naming conventions: - File extensions:.js,.ts,.mjs,.cjs,.jsx, and.tsx [1][2]. - Naming conventions: The files must end with either.spec or.test before the file extension [1][3]. For example, files named login-screen.spec.ts or user-login.test.js would be matched by this default pattern [1][2]. Matching is performed against the absolute file path, and strings provided to the testMatch configuration option are treated as glob patterns [1][3].
Citations:
- 1: https://playwright.dev/docs/api/class-testconfig
- 2: https://playwright.dev/docs/api/class-testproject
- 3: https://github.com/microsoft/playwright/blob/303901d7/docs/src/test-api/class-testconfig.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked globs.js locations:\n'
git ls-files | grep -F 'src/eslint/globs.js' || true
printf '\nRelevant contents:\n'
if [ -f src/eslint/globs.js ]; then
nl -ba src/eslint/globs.js | sed -n '1,80p'
fi
printf '\nSearch for PLAYWRIGHT_SPEC_FILES usage:\n'
rg -n "PLAYWRIGHT_SPEC_FILES|playwrightSpecFiles|playwright" src/eslint || true
printf '\nPackage references:\n'
rg -n '"playwright"|eslint-plugin-playwright|playwright' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
printf '\nParse Playwright default-glob-like extension set vs current glob extensions:\n'
python3 - <<'PY'
from pathlib import Path
import re
txt = Path('src/eslint/globs.js').read_text() if Path('src/eslint/globs.js').exists() else ''
m = re.search(r'PLAYWRIGHT_SPEC_FILES\s*=\s*\[([^]]+)\]', txt)
current = set(re.findall(r'(?<=\\.)[A-Za-z0-9]+(?=[.,}])', m.group(1))) if m else set()
playwright_doc_extensions = {'js','ts','mjs','cjs','jsx','tsx'}
print('current_extensions_in_glob=', sorted(current))
print('doc_extensions=', sorted(playwright_doc_extensions))
print('missing_from_glob=', sorted(playwright_doc_extensions - current))
PYRepository: unraid/js-standards
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Relevant contents:\n'
if [ -f src/eslint/globs.js ]; then
awk '{printf "%6d\t%s\n", NR, $0}' src/eslint/globs.js | sed -n '1,80p'
fi
printf '\nSearch for PLAYWRIGHT_SPEC_FILES usage:\n'
rg -n "PLAYWRIGHT_SPEC_FILES|playwrightSpecFiles|playwright" src/eslint || true
printf '\nPackage references:\n'
for f in package.json package-lock.json pnpm-lock.yaml yarn.lock; do
if [ -f "$f" ]; then
rg -n '"playwright"|eslint-plugin-playwright|playwright' "$f" || true
fi
done
printf '\nCompare current glob extensions with Playwright docs extension list:\n'
python3 - <<'PY'
from pathlib import Path
import re
txt = Path('src/eslint/globs.js').read_text()
m = re.search(r'PLAYWRIGHT_SPEC_FILES\s*=\s*\[([^]]+)\]', txt, re.S)
current = set(re.findall(r'(?<=\\)[A-Za-z0-9]+(?=[.,}])', m.group(1))) if m else set()
doc = {'js','ts','mjs','cjs','jsx','tsx'}
print('current_extensions_in_glob=', sorted(current))
print('doc_extensions=', sorted(doc))
print('missing_from_glob=', sorted(doc - current))
PYRepository: unraid/js-standards
Length of output: 4316
Extend PLAYWRIGHT_SPEC_FILES to match Playwright’s default testMatch.
The document states this glob mirrors Playwright’s default, but Playwright also matches Playwright spec files with .mts, .cts, and .jsx. Add those extensions to keep the pattern and comment consistent; otherwise .mts/.cts/.jsx specs may not get Playwright lint rules.
♻️ Proposed glob extension
-export const PLAYWRIGHT_SPEC_FILES = ["**/*.{spec,test}.{ts,tsx,js,mjs,cjs}"];
+export const PLAYWRIGHT_SPEC_FILES = [
+ "**/*.{spec,test}.{ts,tsx,mts,cts,js,jsx,mjs,cjs}",
+];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Playwright specs — mirrors Playwright's own default `testMatch`. | |
| * | |
| * The plugin's rules key off bare `test`/`expect` identifiers, so in a repo that | |
| * also runs Vitest or Jest under this pattern, re-scope `files` on the spec entry | |
| * to wherever Playwright actually owns (`e2e/**`, `**\/*.e2e.spec.ts`, …). | |
| * Otherwise the rules give Playwright advice about a unit test. | |
| */ | |
| export const PLAYWRIGHT_SPEC_FILES = ["**/*.{spec,test}.{ts,tsx,js,mjs,cjs}"]; | |
| /** | |
| * Playwright specs — mirrors Playwright's own default `testMatch`. | |
| * | |
| * The plugin's rules key off bare `test`/`expect` identifiers, so in a repo that | |
| * also runs Vitest or Jest under this pattern, re-scope `files` on the spec entry | |
| * to wherever Playwright actually owns (`e2e/**`, `**\/*.e2e.spec.ts`, …). | |
| * Otherwise the rules give Playwright advice about a unit test. | |
| */ | |
| export const PLAYWRIGHT_SPEC_FILES = [ | |
| "**/*.{spec,test}.{ts,tsx,mts,cts,js,jsx,mjs,cjs}", | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/eslint/globs.js` around lines 31 - 39, Extend the PLAYWRIGHT_SPEC_FILES
glob’s extension set to include mts, cts, and jsx alongside the existing
TypeScript, JavaScript, and module variants, preserving its current spec/test
filename matching behavior.
| ExportDefaultDeclaration(node) { | ||
| const config = configObject(node.declaration); | ||
| if (!config) return; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The rule ignores CommonJS configs, which the file glob still matches.
PLAYWRIGHT_CONFIG_FILES includes .js and .cjs. A CommonJS Playwright config uses module.exports = defineConfig({...}). The rule listens only for ExportDefaultDeclaration, so such a config passes with no report. The failure is silent: the file is linted and reports nothing.
Add a visitor for the module.exports assignment.
♻️ Proposed handler for `module.exports`
ExportDefaultDeclaration(node) {
const config = configObject(node.declaration);
if (!config) return;
+ checkConfig(config);
+ },
+ "AssignmentExpression > MemberExpression.left"(node) {
+ if (
+ node.object.type !== "Identifier" ||
+ node.object.name !== "module" ||
+ node.computed ||
+ node.property.name !== "exports"
+ ) {
+ return;
+ }
+ const config = configObject(node.parent.right);
+ if (config) checkConfig(config);
+ },Extract the body of the existing visitor into a local checkConfig(config) helper so both entry points share it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/eslint/playwright.js` around lines 82 - 84, Update the rule around
ExportDefaultDeclaration to also inspect CommonJS module.exports assignments
matched by PLAYWRIGHT_CONFIG_FILES. Extract the existing config validation logic
into a local checkConfig(config) helper, then invoke it for both default exports
and module.exports values, including defineConfig({...}) wrappers.
| export default [ | ||
| { | ||
| ...recommended, | ||
| files: PLAYWRIGHT_SPEC_FILES, | ||
| rules: { ...recommended.rules, ...CURATED_SPEC_RULES }, | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate other concerns and check which entries set a TypeScript parser.
fd . src/eslint -e js --exec rg -n 'parser|languageOptions|files:' {} \;
echo "=== README mentions of composing the playwright concern ==="
fd -i readme -e md --exec rg -n -C 3 -i 'playwright' {} \;Repository: unraid/js-standards
Length of output: 1044
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== eslint plugin files ==="
git ls-files src/eslint
echo "=== playwright.js outline/sections ==="
fd -e js playwright.js src/eslint --exec sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1"' sh {}
echo "=== eslint concern files parser context ==="
for f in $(git ls-files src/eslint -e js); do
echo "--- $f"
rg -n 'languageOptions|parser|Files|TS_SOURCE|REACTIVE|VUE|PLAYWRIGHT|recommended|rules:' "$f" -C 2 || true
done
echo "=== exports/config consumers ==="
rg -n "playwright|PLAYWRIGHT_SPEC_FILES|PLAYWRIGHT_CONFIG_FILES|import .*playwright|from .*playwright" .Repository: unraid/js-standards
Length of output: 13824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== eslint plugin files ==="
git ls-files src/eslint
echo "=== playwright.js outline/sections ==="
fd -e js playwright.js src/eslint --exec sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1"' sh {}
echo "=== eslint concern files parser context ==="
for f in $(git ls-files src/eslint -e js); do
echo "--- $f"
rg -n 'languageOptions|parser|files|TS_SOURCE|REACT_FILES|VUE|PLAYWRIGHT|recommended|rules:' "$f" -C 2 || true
done
echo "=== exports/config consumers ==="
rg -n "playwright|PLAYWRIGHT_SPEC_FILES|PLAYWRIGHT_CONFIG_FILES|import .*playwright|from .*playwright" .Repository: unraid/js-standards
Length of output: 13824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test helper ==="
for f in $(git ls-files test -z | xargs -0 -n1 sed -n '1,120p'); do
:
done 2>/dev/null || true
echo "=== README compose sections ==="
sed -n '140,195p' README.md
echo "=== package dependencies relevant ==="
sed -n '1,80p' package.json | rg -n -C 2 'eslint|pnpm|ts|typescript|`@typescript-eslint`|playwright' || trueRepository: unraid/js-standards
Length of output: 5966
🌐 Web query:
@eslint/js 10 recommended config languageOptions parser undefined default parser flat config
💡 Result:
In ESLint 10 (as in previous versions using flat config), the @eslint/js recommended configuration does not define a parser [1][2][3]. Instead, ESLint utilizes a built-in default parser, Espree, when no parser is explicitly specified in the languageOptions property [1]. You are not required to define a parser unless you intend to use a non-default parser (such as @babel/eslint-parser or @typescript-eslint/parser) to handle specific syntax [4][5]. Key points regarding parser configuration in flat config: - Default Behavior: If you omit the languageOptions.parser property, ESLint automatically uses its built-in parser, Espree [1]. - How to Override: If you need a custom parser, you import it and assign it to the languageOptions.parser property within your configuration object [6][4]. - Scope: The @eslint/js recommended configuration (imported from @eslint/js) provides rules, not language parsing settings [1][2][3]. While you often include these rules in the same configuration object as your languageOptions, they remain distinct concerns [1][3]. If you encounter errors suggesting a parser is "undefined" or "required," ensure that your configuration object correctly structures the languageOptions property and verify if any plugins you are using require explicit parser mappings for specific file extensions [6][4][7].
Citations:
- 1: https://eslint.org/docs/latest/use/configure/configuration-files
- 2: https://github.com/eslint/eslint/blob/main/packages/js/README.md
- 3: https://eslint-eslint.mintlify.app/use/configuration
- 4: https://eslint.org/docs/latest/use/configure/parser
- 5: https://eslint.org/blog/2022/08/new-config-system-part-2/
- 6: https://eslint.org/docs/latest/use/configure/migration-guide
- 7: Converting to flat config suddenly requires parser? eslint/eslint#20282
🌐 Web query:
ESLint Espree parser can parse TypeScript TypeScript source parse error
💡 Result:
The ESLint default parser, Espree, does not support TypeScript syntax [1][2][3]. When Espree encounters TypeScript-specific constructs—such as type annotations (e.g., : string), interfaces, or enums—it fails because it is designed only for standard ECMAScript [4][5][3]. Attempting to use Espree on TypeScript files will result in a "Parsing error: Unexpected token" [3]. To resolve this issue, you must configure ESLint to use the @typescript-eslint/parser instead of the default Espree parser [3][6]. This parser is specifically designed to convert TypeScript code into an ESTree-compatible format that ESLint can understand [4][5][7]. To fix the error: 1. Install the @typescript-eslint/parser package. 2. Update your ESLint configuration (e.g., .eslintrc.js or eslint.config.js) to set the parser to @typescript-eslint/parser [6][8]. Example configuration update: parser: '@typescript-eslint/parser' If you require advanced features like type-aware linting, you must also configure the parser to reference your project's TypeScript configuration (tsconfig.json) [8]. Using Espree in a TypeScript project is not supported and will lead to unpredictable behavior and linting errors [9].
Citations:
- 1: Support Flow/TypeScript-type annotations eslint/js#278
- 2: Support Flow/TypeScript-type annotations eslint/js#278
- 3: https://fixdevs.com/blog/eslint-parsing-error-unexpected-token/
- 4: https://github.com/typescript-eslint/typescript-eslint/blob/v8.59.3/docs/packages/Parser.mdx
- 5: https://github.com/typescript-eslint/typescript-eslint/blob/v8.58.1/docs/packages/Parser.mdx
- 6: https://eslint.org/docs/latest/use/configure/parser
- 7: https://www.npmjs.com/package/@typescript-eslint/parser
- 8: https://github.com/typescript-eslint/typescript-eslint/blob/main/docs/getting-started/Typed_Linting.mdx
- 9: Bug: [consistent-type-assertions] Requires parser services, but isn't categorized as such typescript-eslint/typescript-eslint#7624
Document TypeScript composition for the Playwright spec concern.
PLAYWRIGHT_SPEC_FILES includes .ts and .tsx, but the spec entry does not set languageOptions.parser. ESLint falls back to its default parser for .ts/.tsx, which reports parse errors on TypeScript syntax. Add this requirement to the README, or make the spec entry composition-aware by excluding those extensions unless composable with a TypeScript parser concern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/eslint/playwright.js` around lines 156 - 161, Update the Playwright spec
configuration around PLAYWRIGHT_SPEC_FILES so TypeScript and TSX files are not
processed without a compatible TypeScript parser concern; either document and
enforce the required composition in the README or exclude .ts/.tsx from this
entry unless the parser is supplied through composition. Preserve the existing
JavaScript Playwright rules and recommended configuration.
1 participant
Summary
Adds an
eslint/playwrightconcern:eslint-plugin-playwrightfor specs, and alocal rule for the config that runs them.
Why
Playwright defaults
actionTimeoutandnavigationTimeoutto0, which meansno timeout rather than a sensible one.
expect()carries its own default, so aconfig that sets
expect.timeoutreads as bounded — but that ceiling only coversassertions. A bare
.click()or.goto()on an element that never becomesactionable waits out the entire test budget.
On a suite with a 30-minute per-test budget, one unactionable click produced 24
minutes of total silence with the machine completely idle, and the eventual
error named the test rather than the call that hung. With an explicit ceiling in
place the same run failed in ~3 minutes and named the exact selector it was
waiting on. Nothing in the upstream plugin covers this, so the config half is a
local rule.
What's in it
Specs — the plugin's recommended set (conditional logic in tests, forgotten
awaiton assertions,page.waitForTimeoutsleeps, focused/skipped tests leftbehind) plus the tier it ships but leaves out of recommended, all at
error:prefer-to-be,prefer-comparison-matcher,prefer-equality-matcher,prefer-strict-equal,prefer-to-contain,require-to-throw-messagerequire-to-pass-timeout— an unboundedtoPassis the same blackout shape asan unbounded action
no-raw-locators,no-nth-methods,prefer-native-locators,no-get-by-titlerequire-hook,require-top-level-describe,no-slowed-testno-commented-out-testsConfigs (
playwright*.config.*) —playwright-config/require-action-timeoutsrequires both timeouts explicitly. It reads the default export through
defineConfig()and through asatisfies/aswrapper, and reports a config withno
useblock at all, since that inherits both defaults just the same.On severity
These are
errordeliberately. This package is the bar new work is writtenagainst, so the standard states the target rather than the current state of any
one suite. A repo adopting mid-stream downgrades a specific rule in its own
config, which keeps the exception visible and local instead of hidden in the
shared baseline.
Pure convention stays off —
prefer-lowercase-title(style),require-tagsandno-restricted-*(need a project vocabulary),require-soft-assertions(changesfailure semantics),
max-expects(an arbitrary budget), andno-hooks(whichcontradicts
require-hook).Scoping caveat
The plugin's rules key off bare
test/expectidentifiers. A repo that also runsVitest or Jest under
**/*.{spec,test}.*should re-scopefileson the specentry, or the rules will give Playwright advice about a unit test. Documented in
the README.
Verification
npm test— 36 tests, all green; ESLint and Prettier cleanLinter-based conventionfindings on the exact
use:blockparse failures, so adoption is a bounded cleanup rather than a rewrite
Summary by CodeRabbit
New Features
Documentation