-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
attempt to use arm as default for mac #8328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ import type { | |
| Bitness, | ||
| OperatingSystem, | ||
| } from '#site/types/userAgent'; | ||
| import { getHighEntropyValues, detectOS } from '#site/util/userAgent'; | ||
| import { detectOS, getHighEntropyValues } from '#site/util/userAgent'; | ||
|
|
||
| type UserOSState = { | ||
| os: OperatingSystem | 'LOADING'; | ||
|
|
@@ -28,10 +28,12 @@ const useDetectOS = () => { | |
| navigator.userAgent | ||
| ); | ||
|
|
||
| const os = detectOS(); | ||
ovflowd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // We immediately set the OS to LOADING, and then we update it with the detected OS. | ||
| // This is due to that initial render set within the state will indicate a mismatch from | ||
| // the server-side rendering versus what the initial state is from the client-side | ||
| setUserOSState(current => ({ ...current, os: detectOS() })); | ||
| setUserOSState(current => ({ ...current, os })); | ||
|
|
||
| // We attempt to get the high entropy values from the Browser and set the User OS State | ||
| // based from the values we get from the Browser, if it fails we fallback to the User Agent | ||
|
|
@@ -40,9 +42,12 @@ const useDetectOS = () => { | |
| ({ | ||
| // If there is no getHighEntropyValues API on the Browser or it failed to resolve | ||
| // 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Although I'm absolutely not a fan of os specific code, this is a small enough check tho |
||
| // we assume that MacOS has moved to arm64 by default now | ||
| architecture = os === 'MAC' ? 'arm' : 'x86', | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for looking into this - I had a lot of trouble testing this locally and decided to push it for review 😅 |
||
| }) => { | ||
| console.log({ bitness, architecture }); | ||
|
|
||
| setUserOSState(current => ({ | ||
| ...current, | ||
| bitness: bitness as Bitness, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.