attempt to use arm as default for mac #8328
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8328 +/- ##
=======================================
Coverage 76.72% 76.73%
=======================================
Files 118 118
Lines 9805 9808 +3
Branches 335 336 +1
=======================================
+ Hits 7523 7526 +3
Misses 2280 2280
Partials 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the OS detection logic to default to ARM architecture for macOS devices when high entropy values are unavailable. The change reflects the shift in Apple's hardware from Intel (x86) to Apple Silicon (ARM) processors.
- Extracts the
detectOS()call into a variable to avoid calling it twice - Updates the fallback architecture logic to assume ARM for macOS and x86 for other operating systems
- Updates the corresponding test to expect the new default ARM architecture for macOS
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/site/hooks/react-client/useDetectOS.ts | Extracts OS detection to a variable and changes macOS fallback architecture to ARM |
| apps/site/hooks/react-client/tests/useDetectOS.test.mjs | Updates test assertion to expect ARM architecture for macOS |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The code was wrong, there was a bug. I've fixed it. |
| // we attempt to fallback to what the User Agent indicates | ||
| bitness = uaIndicates64 ? '64' : '32', | ||
| architecture = 'x86', | ||
| bitness = os === 'MAC' || uaIndicates64 ? '64' : '32', |
There was a problem hiding this comment.
macOS UA's don't have "64", but 32bit macOS isn't a thing anymore since many generations of macOS/OS X.
There was a problem hiding this comment.
(Although I'm absolutely not a fan of os specific code, this is a small enough check tho
| architecture = 'x86', | ||
| bitness = os === 'MAC' || uaIndicates64 ? '64' : '32', | ||
| // we assume that MacOS has moved to arm64 by default now | ||
| architecture = os === 'MAC' ? 'arm' : 'x86', |
There was a problem hiding this comment.
Also, this was incorrectly set as "arm64", but there's no such thing as "arch64", for some reason TypeScript is resolving as "string"
There was a problem hiding this comment.
thanks for looking into this - I had a lot of trouble testing this locally and decided to push it for review 😅
|
Seems to work fine now (I'm on M1) |
github-actions
bot
removed
the
github_actions:pull-request
|
is there a preview link i can use to verify whether this works or not? |
|
|
Thanks, I didn't see it in the status check section and missed the comment. It certainly defaults to ARM64 for me in Safari on my M4. |
No worries! It's easy T times to get lost on the sea of comments. For future reference, PRs here the Vercel deployment often is the first comment of any PR. |
Description
Set default mac architecture to arm64
Validation
hard to verify as I learned I cannot run the site at all on my work network (a me problem!)
Related Issues
attempt to address #8324
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.