fix: stop mutating the shared user agent cache and serve intel macs the universal launcher - #462
fix: stop mutating the shared user agent cache and serve intel macs the universal launcher#462braianj wants to merge 2 commits into
Conversation
Deploying ui2 with
|
| Latest commit: |
cc30547
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://60fa1d41.ui2-423.pages.dev |
| Branch Preview URL: | https://fix-download-button-os-overr.ui2-423.pages.dev |
decentraland-bot
left a comment
There was a problem hiding this comment.
Review: fix: stop mutating the shared user agent cache and serve intel macs the universal launcher
Verdict: Approve ✅
Excellent fix for two real bugs with clear data-backed impact. The singleton mutation fix is correct and well-guarded, and the Intel Mac universal binary change is straightforward.
Bug 1: Singleton mutation — Fixed correctly
The old setUserAgentArchitectureDefaultByOs mutated the @dcl/hooks module-level _cachedData singleton in place via a useEffect. This leaked the ?os= override to every consumer in the session, with no way to undo it. The new resolveUserAgentDataForOs:
- Returns a new object via shallow spread (
{ ...userAgent, os: { ...userAgent.os, name: os }, cpu: { ...userAgent.cpu, architecture: ... } }) — never mutates the input - Returns the same reference when no override is needed (
!isSupportedOperativeSystem(os)oruserAgent.os.name === os) — safe as auseMemodependency - Gates unsupported OS values via
isSupportedOperativeSystem(strict enum match, case-sensitive) — no injection risk from arbitrary?os=values - Correctly maps macOS →
arm64instead of the oldunknownthat matched no artifact
Bug 2: Intel Mac CDN — Fixed correctly
Both macOS architectures now point to the same LAUNCHER_MACOS_DMG / AUTO_SIGNING_MACOS_DMG constants. The published .dmg is a universal binary (lipo -archs → x86_64 arm64), so Intel Macs run it natively. The LAUNCHER_LEGACY_BASE_URL is removed with no remaining references.
[P2] Minor notes (non-blocking)
-
Deprecated function is justified:
setUserAgentArchitectureDefaultByOsis re-exported viasrc/modules/index.ts, so external npm consumers of@dcl/ui2could import it. Keeping it with@deprecatedrather than deleting is the correct approach for a published package. -
cdnReleases.spec.tsonly tests macOS links: Adding a basic Windows smoke test and identity substitution test for Windows would round out coverage. Not blocking since the Windows config is unchanged. -
?os=windows(lowercase) silently does nothing: TheisSupportedOperativeSystemguard is case-sensitive against the enum values (Windows,macOS). This is intentional — the?os=values come from the "Also available on…" links which use the enum directly. No action needed. -
Import cycle fix is a good bonus:
cdnReleases.tsnow imports fromDownloadButton.typesinstead of the barrel, breaking a cycle that pulled React into the module graph.
Security
No concerns. The os query param is guarded by isSupportedOperativeSystem (only macOS or Windows pass). The value is never used in string interpolation or DOM rendering — it only selects a pre-defined CDN link.
Reviewed by Jarvis 🤖 · Requested by Braian Mellor (<@U03ACDWNHR8>) via Slack
Two independent defects in the download path, found while auditing where the download funnel loses people.
1.
?os=mutated a cache shared by the whole sessionDownloadButtoncalledsetUserAgentArchitectureDefaultByOs(userAgentData, os)inside auseEffect. That function edits its argument in place, and the argument is@dcl/hooks' module-level_cachedDatasingleton, handed to every consumer in the session. Consequences:useMemokeyed on it never recomputes: the rendered icon andhrefstayed on the old platform whilehandleClick, which re-derives from the mutated object, delivered the new one. The button could show the Apple icon and download the.exe.osis present).Also,
?os=macOSnever actually delivered a macOS build: the function setcpu.architecture = 'unknown', and no published artifact has anunknownkey, socreateDownloadOptionreturnednulland the button silently degraded to reopening the download page.Replaced with a pure
resolveUserAgentDataForOs, which returns a new object (the same reference when there is nothing to override, so it stays memo-safe) and resolves macOS toarm64. The query param keeps overriding detection: that is the feature behind the "Also available on…" links and it is intentional.setUserAgentArchitectureDefaultByOsis kept and marked@deprecatedrather than deleted, in case other consumers import it.2. Intel Macs were served the legacy "Decentraland Outdated" build
cdnReleases.tsmapped macOSamd64toDecentraland%20Outdated-mac-x64.dmg(the old Electron launcher) whilearm64got the current one.The current launcher dmg is a universal binary, verified against the published artifact on 2026-08-03:
So Intel Macs run it natively and there is no reason for a separate legacy target. This matches what we measured: Intel Macs were a 100% dead end — 9 anons in July 2026, none of whom ever reached the world. Both architectures now point at one constant so they cannot drift apart again, and on the identity-bound source both use the gateway URL, so Intel downloads keep the baked attribution id instead of falling back to a plain CDN link.
LAUNCHER_LEGACY_BASE_URLis removed (no remaining references).Incidental
cdnReleases.tsimportedOperativeSystemfrom theDownloadButtonbarrel, which pulled the component and React back into this module's graph — a cycle (DownloadButtonimportscdnReleases) and the reason the module could not be unit tested. Now imports fromDownloadButton.typesdirectly, matching whatuserAgent.tsalready did.Verification
Tests are written as pure-function specs to fit this repo's
testEnvironment: 'node'jest setup (no RTL): 13 new tests acrossuserAgent.spec.tsandcdnReleases.spec.ts, including a guard that neither macOS architecture may ever point at an "Outdated" URL again.npx jest— 6 suites, 50 tests greennpx tsc --noEmitcleannpm run lintand prettier clean