Skip to content

Modrinth: project detail + per-server compatibility line - #62

Merged
CaYatur merged 2 commits into
mainfrom
feat/modrinth-detail-compat
Jul 27, 2026
Merged

Modrinth: project detail + per-server compatibility line#62
CaYatur merged 2 commits into
mainfrom
feat/modrinth-detail-compat

Conversation

@CaYatur

@CaYatur CaYatur commented Jul 27, 2026

Copy link
Copy Markdown
Owner

What

Part 2 of the richer Modrinth tab: opening a browse result now shows a detail view with a compatibility line for this specific server. Closes #47 (part 1 was #61).

  • shared/mods.tspickCompatibleVersion(versions, { mcVersion, loaders }) (pure). Filters by loader ∩ MC version, then prefers a stable release over a newer beta/alpha, and takes recency from date_published. A version_number is arbitrary text and is never compared as if it sorted — the same doctrine diffUpdates already follows. A version that declares no loaders / no game_versions is not excluded: claiming "incompatible" on missing metadata is worse than showing it.
  • modrinthDetail() (main-side): title, description body, author, downloads, followers, license, categories, links (Modrinth/source/issues/wiki), version count, plus compatible and latestForLoader. Versions are fetched unfiltered and matched locally, so the UI can distinguish "no build for MC 1.21.4" from "no build for your loader at all" — a server-side filtered query collapses both into an empty list. The members call is .catch(() => []): losing the author must not lose the detail.
  • ModsView: expandable panel per result, green "Compatible — X supports MC Y" or an amber line naming the newest build that does exist. Install from the detail passes that exact version id.

Why the install path changed too

The compatibility line would have lied without it. PR #61 made a hybrid (mohist/arclight) search paper ∪ forge, but installModrinth still queried ?loaders=["paper"] — so a Forge mod could be listed as compatible and then fail with no-compatible-version on click. Both paths now consume the same loader set.

Same fix removes a second-order bug: the target folder was MODDED_TYPES.includes(type) ? 'mods' : 'plugins', and mohist is in both families — every Bukkit plugin installed onto a hybrid server landed in mods/. The folder now comes from the chosen version's own loaders (folderForLoaders), which is the only thing that can decide it for a hybrid.

installMod gained an optional versionId. It is validated against that project's own version list server-side, so the renderer can choose a version but can never point the download at an arbitrary file — same guard applyUpdate uses.

Verification

  • npm run typecheck + npm run build — clean.
  • MSMS_SMOKE_MODUPDATEexit 0. New units: a Paper 1.20.1 server picks the stable paper/spigot build over both a newer Fabric release and a newer Paper beta; an unsupported MC version returns undefined while latestForLoader still resolves; a loader with no builds returns undefined; an empty loader filter excludes nothing; a lexically larger version string loses to the newer date. Plus the hybrid folder decision (forge→mods/, bukkit-family→plugins/, missing→fallback).
  • MSMS_SMOKEexit 0 (renderer still mounts with the rewritten ModsView).
  • en/tr key parity for the mods block: 42/42, no orphans either way.

Disclosed gaps

  • The live Modrinth calls are inspection-only. modrinthDetail is a thin shell over three documented v2 endpoints; the field mapping (icon_url, source_url, license.name, members[].user.username) is read off the API docs, not asserted against a live response in this environment. The pure matching underneath it is what the smoke covers.
  • The detail panel itself is not headless-testable — the smoke asserts the renderer mounts, not that the compatibility line reads correctly on screen.
  • No real install was performed against a hybrid server (no mohist instance here), so the folder fix is verified as a pure decision, not as an observed file landing in plugins/.
  • The project body is rendered as plain pre-wrap text; Modrinth serves markdown, so headings/links appear as raw markup. Deliberate — no markdown renderer is being pulled in for this.

Part 2 of the richer Modrinth tab. Part 1 (PR #61) filtered browse results by
server type; this adds the detail view and, with it, the compatibility verdict.

- shared/mods.ts: pure pickCompatibleVersion(versions, {mcVersion, loaders}).
  Same doctrine as diffUpdates - a version_number is arbitrary text and is never
  compared as if it sorted. Recency comes from date_published, and a stable
  release outranks a newer beta/alpha. Also folderForLoaders(), and PLUGIN_LOADERS
  moved here so the update check and the folder decision cannot drift apart.
- core/mods.ts modrinthDetail(): project + versions + members in one call each,
  versions fetched UNFILTERED so the UI can tell "nothing for your Minecraft
  version" apart from "nothing for your loader". Compatibility is computed
  against searchLoaders(type) - the same set browse was filtered by - so a listed
  result can never claim a compatibility the install then refuses.
- installModrinth(): now picks via the same pure function, accepts an optional
  versionId that is validated against the project's own version list, and takes
  the target folder from the chosen version's loaders instead of the server type.
  A hybrid (mohist/arclight) runs Bukkit plugins AND Forge mods, so type alone
  dropped every plugin into mods/.
- ModsView: expandable detail per result - compatibility line, author, downloads,
  followers, license, categories, description, external links. Late replies are
  discarded if the card was collapsed meanwhile; switching servers clears results
  and detail (they carry the old server's loader verdict).
- en/tr keys in lockstep (42/42).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 27, 2026 20:33

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Two problems found reviewing the branch.

1. toggleDetail ran setDetail/setDetailState inside a setOpenId updater. State
   updaters must be pure - React double-invokes them in StrictMode - and the
   projectId it compared could not tell a re-opened card's new request from its
   own stale one. Replaced with a monotonic request sequence in a ref; close and
   server-switch both bump it, so any in-flight reply is dropped.

2. pickCompatibleVersion is deliberately lenient about a version that declares
   no game_versions, but the UI rendered that as a definite 'X supports MC Y'.
   The line now only names the MC version when the build actually lists it, and
   falls back to the softer 'latest matching version' wording otherwise.
@CaYatur

CaYatur commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

/code-review — self review

Two real findings, both fixed on-branch in becfb77.

1. Side effects inside a state updater, and a stale-reply guard that could not actually tell requests apart — ModsView.tsx

setOpenId((cur) => {
  if (cur === hit.projectId) {
    setDetail(d)        // <- dispatching state from inside an updater
    setDetailState('idle')
  }
  return cur
})

Two things wrong:

  • Updaters must be pure. React double-invokes them under StrictMode, so setDetail/setDetailState fire twice in dev. Benign here, but it is exactly the pattern that stops being benign the moment the updater does anything conditional.
  • The guard did not guard. It compared projectId, which cannot distinguish this request from another request for the same card. Concretely: open a result, collapse it, re-open it before the first fetch settles → two in-flight requests, both matching cur === hit.projectId. The slower one wins and overwrites the newer detail. Rare, but it is a genuine wrong-data-on-screen path, not just a lint complaint.

Replaced with a monotonic detailReq ref. closeDetail() and the server-switch effect both bump it, so a reply that is no longer wanted is dropped rather than compared by identity.

2. The compatibility line claimed an MC version it had never verified — ModsView.tsx

pickCompatibleVersion is intentionally lenient: a version that declares no game_versions is not excluded, because refusing on missing metadata is worse than showing it. That leniency then flowed straight into:

Compatible — 2.4.1 supports MC 1.21.4

…for a build that never said anything about 1.21.4. The whole point of this PR is a compatibility verdict an operator can trust before installing onto a live server, so a definite claim assembled from absent data is the one bug that undermines the feature itself.

Now the "supports MC x.y" wording is used only when compatible.gameVersions.includes(mcVersion); otherwise it degrades to "Latest matching version: X". The leniency stays where it belongs (matching), out of the assertion the UI makes.

Checked and fine

  • installMod's new versionId is re-validated against the project's own version list main-side — a compromised renderer still cannot redirect the download.
  • Server switch clears results + detail; both carried the previous server's loader verdict.
  • Per-card d is open && detail, so a collapsed card can never read another project's detail.
  • members failure degrades to "no author", not to a failed detail.

Re-verified after the fixes: typecheck + build clean, MSMS_SMOKE exit 0, MSMS_SMOKE_MODUPDATE exit 0.

@CaYatur
CaYatur merged commit 40d4141 into main Jul 27, 2026
1 check passed
@CaYatur
CaYatur deleted the feat/modrinth-detail-compat branch July 27, 2026 20:38
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.

Modrinth tab: richer detail, compatible versions, mods-vs-plugins by server type

2 participants