enhancement: better detect apple silicon
Description
Use the snippet from https://github.com/faisalman/ua-parser-js/issues/732#issue-2348848266 to better address #8324. Related to #8328
Validation
Use Safari or Firefox on an M* Mac and see if the download page defaults to arm instead of x64.
Related Issues
- https://github.com/faisalman/ua-parser-js/issues/732#issue-2348848266
- #8324
- #8328
Check List
- [x] I have read the Contributing Guidelines and made commit messages that follow the guideline.
- [x] I have run
pnpm formatto ensure the code follows the style guide. - [x] I have run
pnpm testto check if all tests are passing. - [ ] I have run
pnpm buildto check if the website builds without errors. - [ ] I've covered new added functionality with unit tests if necessary.
The latest updates on your projects. Learn more about Vercel for GitHub.
| Project | Deployment | Preview | Updated (UTC) |
|---|---|---|---|
| nodejs-org | Preview | Nov 5, 2025 5:49am |
Codecov Report
:x: Patch coverage is 48.14815% with 14 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 76.64%. Comparing base (30d6c52) to head (c6ed3d5).
:white_check_mark: All tests successful. No failed tests found.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| apps/site/hooks/react-client/useDetectOS.ts | 48.14% | 14 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #8330 +/- ##
==========================================
- Coverage 76.72% 76.64% -0.09%
==========================================
Files 118 118
Lines 9805 9831 +26
Branches 335 340 +5
==========================================
+ Hits 7523 7535 +12
- Misses 2280 2294 +14
Partials 2 2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I appreciate the effort, Jordan (don't want to discredit it), but this PR is a strong :-1: for me. Opinions, cc @nodejs/nodejs-website?
I mean the solution is hacky indeed, but at least it works, no? 😅 #8328 still shows x64 for me on Safari on an M1 device
I mean the solution is hacky indeed, but at least it works, no? 😅 #8328 still shows
x64for me on Safari on an M1 device
I'll investigate what's wrong on that solution.
I mean the solution is hacky indeed, but at least it works, no? 😅 #8328 still shows
x64for me on Safari on an M1 deviceI'll investigate what's wrong on that solution.
Yeah, now that it works I'd also say the other solution is a better one. Thanks for jumping in
I can certainly add tests. I verified that it returns true in Safari on my M4 Mac. As for "hacky", using a Chrome-only nonstandard feature is FAR more hacky than using standard WebGL to infer architecture information.
I can certainly add tests. I verified that it returns true in Safari on my M4 Mac. As for "hacky", using a Chrome-only nonstandard feature is FAR more hacky than using standard WebGL to infer architecture information.
With all due respect, but, you're absolutely wrong for calling this a Chrome-only non-standard feature (assuming nonstandard as non-specd). It is a spec https://wicg.github.io/ua-client-hints/#navigatoruadata, it's up to the browsers to implement it. Same frigging thing as TC39.
The PR you're providing is a hack around a WebGL flag that by no means is documented (I'm not even sure where to look to their docs) to be used for that purpose and could at any point stop working.
Not to mention, again, this PR breaks the walled-garden of React, and will cause Next.js to eject on SSR due to this manual Element creation.
@ovflowd that link is explicitly to a draft, which means it's not actually standardized yet. To compare to TC39, it'd be like if a feature was shipped prior to stage 3 - relying on it is still inappropriate.
If it's incompatible with next.js, that's one thing, but it shouldn't be incompatible with React whatsoever - the functionality just needs to not be invoked on a server.
@ovflowd that link is explicitly to a draft, which means it's not actually standardized yet. To compare to TC39, it'd be like if a feature was shipped prior to stage 3 - relying on it is still inappropriate.
That's why MDN marks it as experimental. But so is the API you're using, no? But you do have merit here, it's definitely not a "stage 3 proposal" by any means
If it's incompatible with next.js, that's one thing, but it shouldn't be incompatible with React whatsoever - the functionality just needs to not be invoked on a server.
Yes this won't be called on React Server. The issue is SSR (not to be confused with RSC/React Server)
This will both cause Next.js to complain about initial client render mismatching server render and Next.js to bail SSR generation and fallback to CSR.
Hey @ljharb I wanted to publicly apologize as after re-reading my own comments here, I happened to notice that I was unnecessarily hostile here and provocative. Whilst these were not my intentions nor I realized the weight of them at the time of writing, I should have done better.
I absolutely understand you're just trying to offer a solution to us, and as I said before I appreciate you and your effort.
Let me take a step back, I still believe that this PR as it stands in a state that is unmergeable, let's try to change that:
- Can you either verify the license of the code or change the code in a way that is different from the original code so that we don't need to worry too much about it?
- Could you please verify what exactly these WebGL flags are, what they are for and if they absolutely will work in favor of us for what we need?
- Could you document sources for where the original ideas came from and how they were verified?
- Lastly, would you mind adding tests that ensure the happy paths are covered? Since I don't know the real behavior of these flags I don't think that testing unhappy paths would make much sense as these APIs are nevertheless outside of our control.
I do think that @bmuenzenmeyer's PR should be enough, but I'm happy to iterate here if you feel necessary.
Again, I'm really sorry, and I should've done better!
Given that we have an open PR without three blockers, I've decided to close this. If we still see problems with the current implementation post the other PR, we can re-evaluate then
I think closing is premature, since this PR would address the issue more robustly than the open PR, and doesn't block that PR from landing (i think it's already landed?)
No, I don't think it's premature. We've already emphasized that this approach will not land for a number of reasons, so why should we clutter the issue tracker with it?
If you are adamant that this is the way to go, and we are adamant that it's not, this PR won't go anywhere.
To be clear: my only concern with this approach is the license.
Having this here is not cluttering anything if we are all committed to progress and iteration.