Use strict equality in the drop, inventory and sacrifice paths #515

Open

Smithy-bot wants to merge 5 commits from cursor/487-strict-equality into release/0.12.0

Smithy-bot commented

2026-09-02 12:30:06 +01:00

Member

Follow-up 3 of 3 from #487, split out as you asked on the issue. This is the loose-equality item, and it is deliberately partial - please read the scope note before merging.

What changed: 20 comparisons across the five files the checklist calls out by name, == to ===:

File	Comparisons
`src/helpers/InventoryHelper.ts`	7
`src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts`	4
`src/commands/stats.ts`	5
`src/helpers/DropHelpers/GetCardsHelper.ts`	3
`src/commands/sacrifice.ts`	1

Most are length or quantity checks against 0, or page-index checks, where the two sides were already the same type and this is pure hygiene. The ones actually worth a look are the identity comparisons: Inventory.CardNumber === card.id (both declared string) and card.type === rarity / === CardRarity.X (both the numeric CardRarity enum). I checked the declared types on each before changing it, and tsc would have rejected any comparison between genuinely disjoint types, so nothing here silently changes meaning.

The one real risk, stated plainly. Card metadata is JSON.parsed without validation, so card.type is declared a number but nothing stops a metadata file containing "type": "1". Under == that string matched the enum; under === it will not, and the card would quietly drop out of /stats, the drop pool and the inventory filters. I think strict is the correct behaviour - a malformed metadata file should be visible rather than silently coerced - but it is a behaviour change on bad input, not a pure refactor. Worth a yarn build && yarn test plus a real /drop, /inventory and /stats against the production card folder before this goes out. I have added a note to docs/cards.md in #514 asking for parse-level validation of the metadata format.

Scope note: 73 loose comparisons across 37 files remain, and the eqeqeq ESLint rule is not enabled here. Enabling it would fail lint on every file not yet swept, so it has to land with the last slice. I stopped at these five because this repo is only writable through the Forgejo contents API one whole file at a time, which puts a hard ceiling on how much mechanical churn fits in a single pass. Happy to keep going in further slices, or if you would rather do it in one shot, yarn lint:fix with eqeqeq enabled locally will handle the bulk and is a much better use of your time than my API calls.

I also raised separately that the rules block in eslint.config.mjs is scoped to files: ["./src", "./tests"], which matches no files, so eqeqeq will need a correctly scoped block rather than an addition to the existing one.

Verified locally on release/0.12.0 with exactly these five files changed: tsc --noEmit clean, eslint . clean, jest 17/17 suites and 99 tests passing. No overlap with #512, #513 or #514.

Follow-up 3 of 3 from #487, split out as you asked on the issue. This is the loose-equality item, and it is **deliberately partial** - please read the scope note before merging. **What changed:** 20 comparisons across the five files the checklist calls out by name, `==` to `===`: | File | Comparisons | | --- | --- | | `src/helpers/InventoryHelper.ts` | 7 | | `src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts` | 4 | | `src/commands/stats.ts` | 5 | | `src/helpers/DropHelpers/GetCardsHelper.ts` | 3 | | `src/commands/sacrifice.ts` | 1 | Most are length or quantity checks against `0`, or page-index checks, where the two sides were already the same type and this is pure hygiene. The ones actually worth a look are the identity comparisons: `Inventory.CardNumber === card.id` (both declared `string`) and `card.type === rarity` / `=== CardRarity.X` (both the numeric `CardRarity` enum). I checked the declared types on each before changing it, and `tsc` would have rejected any comparison between genuinely disjoint types, so nothing here silently changes meaning. **The one real risk, stated plainly.** Card metadata is `JSON.parse`d without validation, so `card.type` is *declared* a number but nothing stops a metadata file containing `"type": "1"`. Under `==` that string matched the enum; under `===` it will not, and the card would quietly drop out of `/stats`, the drop pool and the inventory filters. I think strict is the correct behaviour - a malformed metadata file should be visible rather than silently coerced - but it is a behaviour change on bad input, not a pure refactor. Worth a `yarn build && yarn test` plus a real `/drop`, `/inventory` and `/stats` against the production card folder before this goes out. I have added a note to `docs/cards.md` in #514 asking for parse-level validation of the metadata format. **Scope note: 73 loose comparisons across 37 files remain**, and the `eqeqeq` ESLint rule is **not** enabled here. Enabling it would fail lint on every file not yet swept, so it has to land with the last slice. I stopped at these five because this repo is only writable through the Forgejo contents API one whole file at a time, which puts a hard ceiling on how much mechanical churn fits in a single pass. Happy to keep going in further slices, or if you would rather do it in one shot, `yarn lint:fix` with `eqeqeq` enabled locally will handle the bulk and is a much better use of your time than my API calls. I also raised separately that the `rules` block in `eslint.config.mjs` is scoped to `files: ["./src", "./tests"]`, which matches no files, so `eqeqeq` will need a correctly scoped block rather than an addition to the existing one. **Verified locally on `release/0.12.0`** with exactly these five files changed: `tsc --noEmit` clean, `eslint .` clean, `jest` 17/17 suites and 99 tests passing. No overlap with #512, #513 or #514.

Smithy-bot added 5 commits

2026-09-02 12:30:06 +01:00

Use strict equality in GetUnclaimedCardsHelper 81ef9d162d