Skip to content

fix(versions): handle invalid semver strings in the version list sort - #154

Merged
Zaldaryon merged 2 commits into
devfrom
fix/listversions-semver-crash
Aug 18, 2026
Merged

fix(versions): handle invalid semver strings in the version list sort#154
Zaldaryon merged 2 commits into
devfrom
fix/listversions-semver-crash

Conversation

@Zaldaryon

Copy link
Copy Markdown
Collaborator

semver.rcompare throws when either argument is not a valid semver string. A registered version with nonstandard -v probe output (pre-release builds, modded launchers, or parse failures) crashed the entire versions page.

What changed

src/renderer/src/features/versions/pages/ListVersions.tsx: replace the bare semver.rcompare() sort with a safe comparator that checks semver.valid() first. Valid versions sort among themselves with rcompare as before. Invalid ones sort last, ordered alphabetically so the list stays deterministic.

What was tested

  • npm run typecheck passes
  • npm run lint:ci passes
  • npm run test:coverage passes (1110 tests)

Fixes #148

semver.rcompare throws when either argument is not a valid semver
string. A registered version with nonstandard probe output (pre-release
builds, modded launchers, or parse failures) crashed the entire versions
page.

Check semver.valid() before comparing. Valid versions sort among
themselves with rcompare as before. Invalid ones sort last, ordered
alphabetically so the list stays deterministic.

Fixes #148
@Zaldaryon
Zaldaryon requested a review from Pixnop August 18, 2026 18:54

@Pixnop Pixnop left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comparator is correct and I checked the property that usually goes wrong in hand-written ones: the signs are symmetric (valid beats invalid from both sides), valids keep the exact old ordering among themselves, and invalids get a deterministic alphabetical tail, so the sort cannot flicker between renders. One thing before approve, and it is a single test. This fixes a white-screen crash, and nothing in the diff fails without the fix: the renderer-dom harness already has versionsListVersions.test.tsx, so seeding one version entry with a nonstandard string (the '1.20' or 'Vintage Story 1.21.0' shapes from issue #148) and asserting the page renders both rows is cheap, fails against dev with the semver throw, and pins the guard. The repo has held every behavior change to that bar for two days, including mine, and a crash fix is the last place to make an exception.

Seed a version entry with a non-parseable string ("Vintage Story 1.21.0")
and assert both rows render. This test fails against dev, where
semver.rcompare throws on the invalid string and crashes the page.
@Zaldaryon

Copy link
Copy Markdown
Collaborator Author

Added in fc99ca5. The test seeds a version entry with "Vintage Story 1.21.0" and asserts both rows render. Against dev (where the sort calls semver.rcompare raw) it throws TypeError: Invalid Version and the page white-screens. Against this branch, both rows render.

@Pixnop Pixnop left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Verified the new test bites: on the branch the file passes 4 for 4, and with only ListVersions.tsx reverted to dev the new case fails on exactly the advertised TypeError, so the white-screen guard is genuinely pinned. Comparator verdict from the first round stands: symmetric signs, old ordering preserved among valids, deterministic tail for invalids.

@Zaldaryon
Zaldaryon merged commit 1c50ad4 into dev Aug 18, 2026
7 checks passed
@Zaldaryon
Zaldaryon deleted the fix/listversions-semver-crash branch August 18, 2026 19:46
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