Skip to content

[utils] Improve platform detection#4920

Open
romgrk wants to merge 19 commits into
mui:masterfrom
romgrk:fix-platform-detection
Open

[utils] Improve platform detection#4920
romgrk wants to merge 19 commits into
mui:masterfrom
romgrk:fix-platform-detection

Conversation

@romgrk
Copy link
Copy Markdown
Contributor

@romgrk romgrk commented May 26, 2026

Summary

The previous detectBrowser.ts exported a flat list of booleans (isWebKit, isFirefox, isMac, isIOS, isAndroid, isJSDOM) that conflated OS, rendering engine, and environment into one undifferentiated namespace. Several call sites had picked the wrong flag because the names suggest a single coherent category that doesn't actually exist:

  • "Browser" is imprecise. All iOS browsers (Chrome, Firefox, Edge on iOS) use WKWebView, so they're WebKit at the engine level regardless of the user-facing brand. A check that's really about "Safari macOS focus quirk" should not also fire for GNOME Web on Linux; a check that's really about "VoiceOver virtual cursor" should not also fire for WebKitGTK + Orca.
  • isFirefox was a brand, not an engine. Firefox-on-iOS is WebKit (its UA marker is FxiOS/), so checks like "Firefox text-direction bug" need to target the Gecko engine specifically, not the Firefox brand.
  • OS vs. form factor were tangled. isIOS was being used both as a true OS check (iOS-only software keyboard behavior) and as a "touch is primary input" proxy — those are different things and call for different signals.
  • isWebKit was overloaded. Some sites meant "WebKit rendering bug" (CSS, layout), others meant "VoiceOver accessibility quirk" (Apple platform + WebKit). Conflating them made one or the other site wrong on platforms like WebKitGTK.

The rule going forward: a quirk patch should name the thing it's actually patching — an OS behavior, an engine bug, an input modality, or an accessibility-tree quirk. Never "this browser".

Changes

Old (@base-ui/utils/detectBrowser) — flat booleans:

import { isWebKit, isMac, isIOS, isAndroid, isFirefox, isJSDOM } from '@base-ui/utils/detectBrowser';

New (@base-ui/utils/platform) — single grouped object:

import { platform } from '@base-ui/utils/platform';

platform.os.mac
platform.os.ios
platform.os.android
platform.os.windows
platform.os.linux
platform.os.apple          // mac || ios

platform.engine.webkit     // Safari, all iOS browsers, GNOME Web (NOT Blink)
platform.engine.blink      // Chrome, Edge, Opera, Brave, …
platform.engine.gecko      // Firefox (desktop / Android — Firefox iOS is WebKit)

platform.pointer.hover     // device has hover capability (desktop, hybrid laptop) — defaults to `true` in SSR
platform.pointer.touch     // device can dispatch touch events (`Touch` / `TouchEvent` constructors exist)
platform.pointer.coarse    // any coarse pointer is available

platform.screenReader.voiceOver  // Apple OS + WebKit (the AX path with the VoiceOver virtual-cursor quirk)

platform.env.jsdom         // jsdom or HappyDOM (tests)

Migration

Old New
isWebKit (engine bug) platform.engine.webkit
isWebKit (VoiceOver workaround) platform.screenReader.voiceOver
isFirefox platform.engine.gecko
isMac platform.os.mac
isIOS platform.os.ios
isAndroid platform.os.android
isJSDOM (source code) platform.env.jsdom
isJSDOM (tests) import { isJSDOM } from '#test-utils'

Notable specifics

  • FocusGuard previously gated its role="button" workaround on isWebKit, which was over-broad — on WebKitGTK + Orca the workaround was setting a meaningless role on a hidden span. Now gated on platform.screenReader.voiceOver, which is (os.mac || os.ios) && engine.webkit — the only place where VoiceOver's NSAccessibility virtual-cursor path is active.
  • useDismiss.ts was importing a function-form isWebKit() from @floating-ui/utils/dom while the rest of the codebase used the boolean from detectBrowser. Unified on platform.engine.webkit.
  • Engines are mutually exclusive by construction. gecko and blink are both anchored to !webkit, so Firefox-on-iOS (FxiOS/, WebKit) and Chrome-on-iOS (CriOS/, WebKit) classify correctly as WebKit-not-Gecko/Blink regardless of future UA-string drift.
  • pointer.hover and pointer.touch are orthogonal. hover is a modality check (derived from (hover: none)); touch is a capability check (Touch / TouchEvent constructor presence). A hybrid laptop with a touchscreen reads hover: true, touch: true; a phone reads hover: false, touch: true; a desktop reads hover: true, touch: false. Use hover to choose UX (hover-vs-tap), touch to decide whether to bind touch handlers.
  • SSR defaults. Every flag resolves to false when navigator is undefined, except pointer.hover, which defaults to true (assume desktop).
  • isJSDOM now also matches HappyDOM, matching the equivalent helper in @mui/x-internals/platform.

Breaking change

@base-ui/utils/detectBrowser is removed. Consumers must import from @base-ui/utils/platform and update field references per the migration table above. vi.mock('@base-ui/utils/platform', …) users must mock the nested platform object shape rather than individual booleans.

@romgrk romgrk added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. internal Behind-the-scenes enhancement. Formerly called “core”. package: utils Specific to the utils package. labels May 26, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 26, 2026

commit: 3b2154b

@code-infra-dashboard
Copy link
Copy Markdown

code-infra-dashboard Bot commented May 26, 2026

Bundle size

Bundle Parsed size Gzip size
@base-ui/react ▼-355B(-0.08%) ▼-76B(-0.05%)

Details of bundle changes

Performance

Total duration: 1,086.81 ms -104.56 ms(-8.8%) | Renders: 50 (+0) | Paint: 1,652.45 ms -169.88 ms(-9.3%)

No significant changes — details


Check out the code infra dashboard for more information about this PR.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 27, 2026

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 793c25b
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6a174e7a6068a60009e18931
😎 Deploy Preview https://deploy-preview-4920--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 27, 2026

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 2f1a92b
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6a1a1cbdc8575f00080a3605
😎 Deploy Preview https://deploy-preview-4920--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@romgrk romgrk added the breaking change Introduces changes that are not backward compatible. label May 27, 2026
@romgrk romgrk marked this pull request as ready for review May 27, 2026 22:34
const [fallbackToCodeSandbox, setFallbackToCodeSandbox] = React.useState(false);
React.useEffect(() => {
if (isSafari || isEdge) {
if (platform.engine.webkit) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if Edge works with stackblitz.

import { FloatingUIOpenChangeDetails } from '../../internals/types';

const isMacSafari = isMac && isSafari;
const isMacSafari = platform.os.mac && platform.engine.webkit;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flag name could potentially be inaccurate.

@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented May 27, 2026

Most use of platform are usually considered a code smell since they don't age well; so we should rely on feature detection whenever possible. So, could we systematically add a comment on why we can't use feature detection in each case?

@romgrk
Copy link
Copy Markdown
Contributor Author

romgrk commented May 28, 2026

I think we need both concerns:

  • Workarounds for bugs/quirks (OS, engine, screen reader)
  • Feature detection

But yes feature detection should be the default, and we should be very precise about what we're detecting, a flag like isMobile is going to be wrong in some cases (either logically or semantically) because it conflates many things (pointer type, screen size, touch support, keyboard).

I've added more pointer categories, because many combinations of hover: boolean, touch: boolean, coarse: boolean are possible, the most confusing ones:

  • a mobile device can use a pen pointer: hover: false, touch: true, coarse: false
  • a laptop can have a touch screen: hover: true, touch: true, coarse: false
  • a device with a remote pointer (wii) is: hover: true, touch: false, coarse: true
    And some devices are pretty fluid (e.g. iPad can be used as a tablet or as a desktop, in which case it's going to pretend to be a mac).

@atomiks
Copy link
Copy Markdown
Contributor

atomiks commented May 28, 2026

My main concern is the bundle size increase here, especially for a change not critical to the lib. I've been trying to reduce the overall size of the library but size increases keep returning over time and I'd rather not implement a noticeable increase for just a tidy-up.

Moving from individual booleans to one exported platform object makes every consumer pull in all detections, even when a call site only needs one flag like platform.engine.webkit. That also includes the new pointer.* checks, which are not used anywhere in this PR yet.

A better shape would be a namespace export that remains treeshakable. The grouped object reads nicely, but it turns the utility into an all-or-nothing import, which feels too expensive for this kind of helper.

@romgrk
Copy link
Copy Markdown
Contributor Author

romgrk commented May 28, 2026

It's not just a tidy-up, I've seen some issues across our repositories where a flag/check would produce incorrect results for specific environments, it's also a correctness change.

That also includes the new pointer.* checks, which are not used anywhere in this PR yet.

Note that this module (like all @base-ui/utils) is meant for use in all our products. That part covers checks that aren't in this repository. Other codebases use partial copies or equivalents of detectBrowser.ts, I'm aiming to centralize & standardize platform detection across all of them.

My main concern is the bundle size increase

So you'd suggest breaking the object into its direct fields, which would be exported directly? I also considered that, although I felt like readability was a bit less optimal. Also, os collides with the node:os module which is usually imported as os. I guess it would be fairly rare for those two modules to be used in the same file though. Please let me know what you prefer.

@romgrk
Copy link
Copy Markdown
Contributor Author

romgrk commented May 28, 2026

I've added a few commits to improve bundle-size, one of them being avoiding the brands in prod. IIUC, that was meant to avoid a devtools warning so it's not relevant to keep it in the bundle in prod.

From a quick search, it seems that bundlers can tree-shake import * as x imports, so we could keep optimal readability & bundle size by using this pattern, although import * is not something we usually do in our codebases. Thoughts?

import * as platform from '@base-ui/utils/platform'
// ...
if (platform.os.ios) { /* ... */ }

@aarongarciah
Copy link
Copy Markdown
Member

+1 in favor of using import * as platform instead of an object. AI (and humans) will use whatever is used in the code base (we can also add something to AGENTS.md), so I don't think it's a problem.

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 28, 2026
@atomiks
Copy link
Copy Markdown
Contributor

atomiks commented May 29, 2026

@romgrk you can check reason-parts.ts and reasons.ts. They group them into a namespace export so you can just do import { platform } from '...' and it stays treeshakeable, like our components do. But this doesn't treeshake the object parts; ideally you'd namespace the exports like import { engine } so you don't have two levels of nesting.

@romgrk romgrk marked this pull request as draft May 29, 2026 22:55
@romgrk romgrk marked this pull request as ready for review May 29, 2026 23:09
@romgrk
Copy link
Copy Markdown
Contributor Author

romgrk commented May 29, 2026

I have applied the suggestions as well as went a bit overboard with bundle-size micro-optimization by avoiding duplicate inline strings and abusing regexes. The combined result is that PR now decreases bundle-size.

Comment on lines +5 to +7
/** iPhone, iPad (including iPadOS 13+ reporting as macOS), iPod. */
export const ios =
/^i(os$|p)/.test(lowerPlatform) || (lowerPlatform === 'macintel' && maxTouchPoints > 1);
Copy link
Copy Markdown
Contributor Author

@romgrk romgrk May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second check ((lowerPlatform === 'macintel' && maxTouchPoints > 1) will become incorrect as Apple plans to ship a mac latop with a touchscreen. This needs either strenghtening or abandoning.

If iOS wants to pretend to be a macOS, we should probably respect that. iPads can be used as a laptop with keyboard & mouse plugged.

If we were using ios as a proxy for "primary pointer is touch", we probably need to adjust those sites to use pointer. Also, pointer needs refinement because we'd need a flag with a semantic "primary pointer is touch", right now we only have "there is a pointer with touch capability" (includes laptops with touchscreen).

Thoughts on dropping the second check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Introduces changes that are not backward compatible. internal Behind-the-scenes enhancement. Formerly called “core”. package: utils Specific to the utils package. PR: out-of-date The pull request has merge conflicts and can't be merged. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants