Skip to content

[Refactor] Simplify severity counting in countOffenseTypes - #8552

Merged
gonzaloriestra merged 1 commit into
mainfrom
refactor-maintenance-34913123511
Sep 16, 2026
Merged

gonzaloriestra merged 1 commit into
mainfrom
refactor-maintenance-34913123511

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

countOffenseTypes in packages/theme/src/cli/services/check.ts counted offenses per severity in three steps: an Object.prototype.hasOwnProperty.call probe, an explicit = 0 initialization, and a !-asserted increment. The non-null assertion was only safe because of the preceding initialization, so the reader has to follow all three lines to convince themselves the counter is sound.

WHAT is this pull request doing?

Collapse the accumulator update into a single nullish-coalescing expression, so the "start at zero, then add one" intent reads directly and the non-null assertion is no longer needed:

acc[offense.severity] = (acc[offense.severity] ?? 0) + 1

This matches the counting idiom already used elsewhere in the repo (for example packages/plugin-did-you-mean/src/index.ts).

Behavior is unchanged. The accumulator starts as {} and only ever stores numbers, so no key is ever explicitly set to undefined — the ?? 0 branch is taken in exactly the cases where the hasOwnProperty probe previously returned false.

How to manually test your changes?

pnpm shopify theme check

Run against a theme containing errors, warnings, and info-level offenses, and confirm the summary counts are unchanged. Also check the JSON shape:

pnpm shopify theme check --json

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Replace the hasOwnProperty pre-initialization with nullish coalescing,
matching the counting idiom used elsewhere in the repo.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@Suleimanlatrsh Suleimanlatrsh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, including repeated severities and the text/JSON output. Looks good 👍

@gonzaloriestra
gonzaloriestra marked this pull request as ready for review September 16, 2026 08:56
@gonzaloriestra
gonzaloriestra requested a review from a team as a code owner September 16, 2026 08:56
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Sep 16, 2026
@gonzaloriestra
gonzaloriestra added this pull request to the merge queue Sep 16, 2026
Merged via the queue into main with commit 410ba42 Sep 16, 2026
28 of 29 checks passed
@gonzaloriestra
gonzaloriestra deleted the refactor-maintenance-34913123511 branch September 16, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants