You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The eslint-plugin-unicorn 64 → 66 bump (#276) enabled a batch of new recommended rules. The cleanly autofixable ones were applied in that PR and remain errors. To land the bump without a mass manual rewrite, the rules below were temporarily downgraded to warnings in extension/eslint.config.js. This issue tracks ratcheting them back to "error" (or fixing the underlying code), one rule per PR.
As we ratchet, we've found some rules are mostly false positives in this codebase and aren't worth enabling — those are split into their own section below so we don't keep re-investigating them.
Done in Ratchet unicorn/prefer-number-coercion to error #296.unicorn/prefer-number-coercion — verified per-site: 13 bare-numeric-token sites converted to Number(); 2 sites in hidden-text-strip.ts keep parseFloat behind a scoped disable (unit-suffixed CSS durations like "0.5s", and a possibly-"" computed opacity — both of which Number() mis-coerces).
Turned off in Disable 5 unicorn rules that only fire on intentional patterns #288.unicorn/no-top-level-side-effects (1, effective-enforcement.ts:115) — the single site is a deliberate, documented module-load registerMethods({...}). Enabling means an inline disable. Low value.
❌ Not worth ratcheting — keep as warn (or turn off)
unicorn/prefer-scoped-selector (64) — turned off in Disable unicorn/prefer-scoped-selector (#279) #297. The rule's :scope rewrite is unsound for our dominant query-receiver type. It assumes the receiver is an Element, where el.querySelectorAll("foo") ≡ el.querySelectorAll(":scope foo"). But the shadow-piercing scanners take root: ParentNode and run against Document, ShadowRoot, and DocumentFragment receivers at runtime, where :scope is not equivalent (verified in jsdom): on a ShadowRoot/DocumentFragment :scope matches nothing, so :scope * / :scope noscript silently return empty; on a Document :scope * excludes <html>; comma-lists break partially (:scope p, span → 1 vs 2). Mechanically applying the suggestion would silently disable defenses inside shadow DOM (noscript-strip, hidden-text-strip, unicode-invisibles-strip, trust-badge-annotate, closed-shadow-root-annotate, …). The Element-receiver sites where :scopeis safe are all single-compound selectors with no descendant combinator, so there's no correctness upside there either. The rule can't narrow to Element receivers or combinator selectors. Worse than no rule: it would lead a developer to break a query.
unicorn/prefer-uint8array-base64 (8) — turned off in Disable unicorn/prefer-uint8array-base64 #295. Blocked by the test runtime, not the codebase. The suggested Uint8Array.fromBase64() / #toBase64() exist in the Chrome target (133+; manifest requires 148) and in Bun (the build script), but Node 24 — which runs Jest/jsdom in CI — only exposes them behind the experimental --js-base-64 flag, so they're absent under test. The 8 sites include two security-relevant decode paths (encoded-payload-redact.ts, encoded-fixture.ts) plus their fixtures; rewriting them would either break the suite or force an atob/btoa-based polyfill into jsdom that exercises the shim instead of Chrome's native impl, masking prod divergence on a decode path. Not worth it for a stylistic rule. Revisit once Node ships these unflagged.
unicorn/max-nested-calls (16) — turned off in Disable unicorn/max-nested-calls #294. Fires only on intentionally deep nesting: zod schema builders (z.record(z.string(), z.union(…))) and fast-check generators (fc.array(fc.tuple(…))). Both read better nested than split.
unicorn/no-this-outside-of-class (30) — turned off in Disable unicorn/no-this-outside-of-class #287. Only fires on legitimate this-outside-a-class (native-prototype monkeypatches, defineProperty accessors, executeScriptthis: Window entry fns, webext-messaging handlers) — none rewritable as classes. Pure noise.
unicorn/no-break-in-nested-loop (27) — turned off in Disable unicorn/no-break-in-nested-loop (#279) #298. Not a clean ratchet. The rule treats a switch inside a loop as a "nested loop", so ~9 sites are idiomatic switch-case breaks (required syntax, not loop control) it would force into pointless function extractions; the rest are test/mock code where extraction is awkward. The rule has no option to exclude switch or narrow scope. The genuinely improvable production sites were cleaned up directly in Disable unicorn/no-break-in-nested-loop (#279) #298 (guard inversions, .some(), and small helper extractions in subtree-watcher.ts, rule-count.ts, schema-trust-sanitize.ts, encoded-payload-redact.ts, disguised-ad-flag.ts, build.ts, selector-token-index.ts, countdown-timer-redact.ts) without enabling the rule.
Investigated and determined to be mostly false positives here. Recommend leaving as warnings (still flags genuinely new bad code) or turning off to cut lint noise. Not a straight ratchet.
Turned off in Disable 5 unicorn rules that only fire on intentional patterns #288.unicorn/prefer-await (41) — all sites are the intentional fire-and-forget void promise.then/.catch() idiom the codebase uses to satisfy no-floating-promises, in contexts that can't cleanly take await (void-returning sync fns, React useEffect/event-listener/setTimeout callbacks, and classic-script top-level — content.ts already disables prefer-top-level-await there). The rule is all-or-nothing, so enabling it would mean ~20 void (async () => { try { … } catch { … } })() rewrites across background/content code — uglier, riskier (error/ordering/abort semantics), no benefit. Keep as warn so it still flags genuinely new sequential .then chains.
Turned off in Disable 5 unicorn rules that only fire on intentional patterns #288.unicorn/no-computed-property-existence-check (23) — fires on anyobj[dynamicKey] in a boolean position, not just existence checks. ~21 of 23 are legitimate value access (!RULE_DEFAULTS[id], if (states[rule.id]), targetMask[i]); only 2 are genuine key in obj existence checks where Object.hasOwn() would help. The rule has no options to narrow scope, and switching the idempotency-flag guards in the injected *-source.ts defenses to Object.hasOwn would let a hostile page pre-spoof the flag. (The 2 genuine Object.hasOwn wins — schema-trust-sanitize.ts, catalog.test.ts — can be done as a tiny standalone change without enabling the rule.)
unicorn/filename-case set checkDirectories: false — v66 added directory-name checking that flags the Jest __tests__ / __test-mocks__ convention we keep.
The
eslint-plugin-unicorn64 → 66 bump (#276) enabled a batch of new recommended rules. The cleanly autofixable ones were applied in that PR and remain errors. To land the bump without a mass manual rewrite, the rules below were temporarily downgraded to warnings inextension/eslint.config.js. This issue tracks ratcheting them back to"error"(or fixing the underlying code), one rule per PR.As we ratchet, we've found some rules are mostly false positives in this codebase and aren't worth enabling — those are split into their own section below so we don't keep re-investigating them.
✅ Completed
unicorn/better-dom-traversingunicorn/prefer-number-is-safe-integerunicorn/prefer-includes-over-repeated-comparisonsunicorn/prefer-type-literal-lastunicorn/no-global-object-property-assignment(error in prod, off in tests)unicorn/no-declarations-before-early-exitunicorn/prefer-short-arrow-methodunicorn/prefer-iterator-to-array(addedESNext.Iteratorto tsconfig lib)unicorn/no-incorrect-query-selector(1 scoped disable)unicorn/require-array-sort-compare(comparators for dynamic arrays; disables for constant catalog lists)unicorn/prefer-minimal-ternary(1 scoped disable for an instanceof-narrowed ternary)unicorn/no-unreadable-new-expression(extractednew X()to locals; parens get stripped by Biome)unicorn/no-unnecessary-global-this(droppedglobalThis.on prod Web-API calls; off in tests for mocking)unicorn/prefer-number-coercion(13 sites →Number(); 2 scoped disables inhidden-text-strip.tswhereparseFloatis load-bearing)Remaining — ratchet candidates
None — all candidates resolved. ✅
Ratchet candidates needing care (not clean)
unicorn/prefer-number-coercion— verified per-site: 13 bare-numeric-token sites converted toNumber(); 2 sites inhidden-text-strip.tskeepparseFloatbehind a scoped disable (unit-suffixed CSS durations like"0.5s", and a possibly-""computedopacity— both of whichNumber()mis-coerces).unicorn/no-top-level-side-effects(1,effective-enforcement.ts:115) — the single site is a deliberate, documented module-loadregisterMethods({...}). Enabling means an inline disable. Low value.❌ Not worth ratcheting — keep as
warn(or turnoff)unicorn/prefer-scoped-selector(64) — turned off in Disable unicorn/prefer-scoped-selector (#279) #297. The rule's:scoperewrite is unsound for our dominant query-receiver type. It assumes the receiver is anElement, whereel.querySelectorAll("foo")≡el.querySelectorAll(":scope foo"). But the shadow-piercing scanners takeroot: ParentNodeand run againstDocument,ShadowRoot, andDocumentFragmentreceivers at runtime, where:scopeis not equivalent (verified in jsdom): on a ShadowRoot/DocumentFragment:scopematches nothing, so:scope */:scope noscriptsilently return empty; on a Document:scope *excludes<html>; comma-lists break partially (:scope p, span→ 1 vs 2). Mechanically applying the suggestion would silently disable defenses inside shadow DOM (noscript-strip,hidden-text-strip,unicode-invisibles-strip,trust-badge-annotate,closed-shadow-root-annotate, …). The Element-receiver sites where:scopeis safe are all single-compound selectors with no descendant combinator, so there's no correctness upside there either. The rule can't narrow to Element receivers or combinator selectors. Worse than no rule: it would lead a developer to break a query.unicorn/prefer-uint8array-base64(8) — turned off in Disable unicorn/prefer-uint8array-base64 #295. Blocked by the test runtime, not the codebase. The suggestedUint8Array.fromBase64()/#toBase64()exist in the Chrome target (133+; manifest requires 148) and in Bun (the build script), but Node 24 — which runs Jest/jsdom in CI — only exposes them behind the experimental--js-base-64flag, so they're absent under test. The 8 sites include two security-relevant decode paths (encoded-payload-redact.ts,encoded-fixture.ts) plus their fixtures; rewriting them would either break the suite or force anatob/btoa-based polyfill into jsdom that exercises the shim instead of Chrome's native impl, masking prod divergence on a decode path. Not worth it for a stylistic rule. Revisit once Node ships these unflagged.unicorn/max-nested-calls(16) — turned off in Disable unicorn/max-nested-calls #294. Fires only on intentionally deep nesting: zod schema builders (z.record(z.string(), z.union(…))) and fast-check generators (fc.array(fc.tuple(…))). Both read better nested than split.unicorn/no-this-outside-of-class(30) — turned off in Disable unicorn/no-this-outside-of-class #287. Only fires on legitimatethis-outside-a-class (native-prototype monkeypatches,definePropertyaccessors,executeScriptthis: Windowentry fns, webext-messaging handlers) — none rewritable as classes. Pure noise.unicorn/no-break-in-nested-loop(27) — turned off in Disable unicorn/no-break-in-nested-loop (#279) #298. Not a clean ratchet. The rule treats aswitchinside a loop as a "nested loop", so ~9 sites are idiomatic switch-casebreaks (required syntax, not loop control) it would force into pointless function extractions; the rest are test/mock code where extraction is awkward. The rule has no option to excludeswitchor narrow scope. The genuinely improvable production sites were cleaned up directly in Disable unicorn/no-break-in-nested-loop (#279) #298 (guard inversions,.some(), and small helper extractions insubtree-watcher.ts,rule-count.ts,schema-trust-sanitize.ts,encoded-payload-redact.ts,disguised-ad-flag.ts,build.ts,selector-token-index.ts,countdown-timer-redact.ts) without enabling the rule.Investigated and determined to be mostly false positives here. Recommend leaving as warnings (still flags genuinely new bad code) or turning off to cut lint noise. Not a straight ratchet.
unicorn/prefer-await(41) — all sites are the intentional fire-and-forgetvoid promise.then/.catch()idiom the codebase uses to satisfyno-floating-promises, in contexts that can't cleanly takeawait(void-returning sync fns, ReactuseEffect/event-listener/setTimeoutcallbacks, and classic-script top-level —content.tsalready disablesprefer-top-level-awaitthere). The rule is all-or-nothing, so enabling it would mean ~20void (async () => { try { … } catch { … } })()rewrites across background/content code — uglier, riskier (error/ordering/abort semantics), no benefit. Keep aswarnso it still flags genuinely new sequential.thenchains.unicorn/no-computed-property-existence-check(23) — fires on anyobj[dynamicKey]in a boolean position, not just existence checks. ~21 of 23 are legitimate value access (!RULE_DEFAULTS[id],if (states[rule.id]),targetMask[i]); only 2 are genuinekey in objexistence checks whereObject.hasOwn()would help. The rule has no options to narrow scope, and switching the idempotency-flag guards in the injected*-source.tsdefenses toObject.hasOwnwould let a hostile page pre-spoof the flag. (The 2 genuineObject.hasOwnwins —schema-trust-sanitize.ts,catalog.test.ts— can be done as a tiny standalone change without enabling the rule.)Rules turned OFF in #276 (decide separately)
unicorn/comment-content— rewrites prose and real paths in comments (.github/…→.GitHub/…,*.yaml→*.YAML,Vue→Vue.js).unicorn/prefer-https— rewritesschema.orgitemtype URIs and intentionalhttp://test fixtures.unicorn/require-css-escape— double-escapes values already run throughescapeAttributeValueand breaks on numeric interpolations.Also configured in #276 (permanent, no action)
unicorn/filename-casesetcheckDirectories: false— v66 added directory-name checking that flags the Jest__tests__/__test-mocks__convention we keep.