Skip to content

fix: resolve lint violations blocking xo v4 / ava v8 upgrade - #8

Merged
mstuart merged 1 commit into
masterfrom
fix/ci-repair
Aug 4, 2026
Merged

fix: resolve lint violations blocking xo v4 / ava v8 upgrade#8
mstuart merged 1 commit into
masterfrom
fix/ci-repair

Conversation

@mstuart

@mstuart mstuart commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Root cause

The open dependabot PR (#6) bumping xo (1.2.3 -> 4.0.0) and ava (7.0.0 -> 8.0.1) has been failing CI on all Node versions, which is why the automated merge daemon has been skipping it (and would skip any future grouped dependency-update PR touching xo) for weeks.

xo 4.0.0 pulls in newer eslint-plugin-unicorn/eslint-plugin-jsdoc rules that flag pre-existing patterns already present in index.js/index.d.ts on master. Because package.json pins xo to ^1.2.3, master's own CI never sees these rules and stays green — but the moment any PR bumps xo past that range, the new rules fire against unrelated, already-merged code:

  • unicorn/no-duplicate-if-branches x2 — the array/object branches of diff() had identical bodies
  • unicorn/no-computed-property-existence-check x2 — key in object checks
  • unicorn/prefer-continue — a whole loop body wrapped in one if
  • unicorn/no-break-in-nested-loop x3 — break inside a switch nested in patch()'s for loop
  • jsdoc/informative-docs — a @param description that just repeated the param name

Fix

Refactored the flagged spots without changing behavior:

  • Merged the duplicate object/array branches in diff() into a single condition (they already called the same recursive diff() regardless of array vs. object).
  • Replaced key in object with Object.hasOwn(object, key).
  • Converted the removal-pass if into an early continue.
  • Extracted patch()'s switch into a standalone applyOperation() helper so its break statements are no longer nested inside the outer loop.
  • Reworded the repetitive JSDoc description.

This is a source-only fix — package.json is untouched, since the actual dependency bump belongs to the dependabot PR, not this one.

Verification

  • npm test (xo && ava && tsd) passes clean against the currently pinned versions (xo@1.2.3, ava@7.0.0) — only a pre-existing, non-blocking complexity warning remains.
  • Also verified against the exact versions the open dependabot PR bumps to (xo@4.0.0, ava@8.0.1): all 9 previously-failing lint errors are gone, all 34 ava tests pass unmodified, tsd passes.

Note (separate issue, not touched here)

.github/workflows/release.yml triggers push: branches: [main], but this repo's default branch is master — so the release workflow never runs. Flagging for awareness; out of scope for this CI fix.

The open dependabot PR bumping xo (1.2.3 -> 4.0.0) and ava (7.0.0 -> 8.0.1)
fails CI because xo 4.0.0 pulls in newer eslint-plugin-unicorn rules that
flag pre-existing patterns in this repo's source:

- Duplicate if/else-if branches in diff() (object vs array cases shared
  an identical body) -> merged into one condition.
- `key in object` existence checks -> replaced with Object.hasOwn().
- A whole-loop-body if in the removal pass -> converted to an early
  continue.
- break statements inside a switch nested in patch()'s for loop ->
  switch extracted into a standalone applyOperation() helper.
- A JSDoc @param description that just repeated the param name.

None of this changes runtime behavior (all 34 existing tests still pass
unmodified); it just brings the source in line with the stricter lint
rules any future xo major bump will apply. This has been blocking every
grouped dependency-update PR the merge daemon looks at, so it belongs on
master rather than on the individual dependabot branches.
Comment thread index.js
switch (operation.op) {
case 'add':
case 'replace': {
parent[key] = operation.value;
Comment thread index.js
if (Array.isArray(parent)) {
parent.splice(key, 1);
} else {
delete parent[key];
@mstuart
mstuart merged commit fc98801 into master Aug 4, 2026
9 checks passed
@mstuart
mstuart deleted the fix/ci-repair branch August 4, 2026 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants