Consolidate client user agent parsing #103048

aduth · 2025-05-01T14:39:06Z

Related to #103023 (in turn, #102984 (comment))

Proposed Changes

Removes express-useragent usage in client libraries, consolidating to use a single library for user agent parsing (ua-parser-js)

Why are these changes being made?

Unify approaches for user agent parsing
Work toward eliminating a dependency to reduce maintenance overhead and supply chain attack risk
Potential performance improvement by deferring user-agent parsing to be just-in-time, rather than computed at the time of module load

Testing Instructions

Verify that relevant components using parsed user agent details continue to behave as expected.

Verify that newly-added tests pass:

yarn test-client client/lib/user-agent/test

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2025-05-01T14:39:19Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-142842

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-142842&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-142842&env=a8c-for-agencies

matticbot · 2025-05-01T14:46:05Z

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~391 bytes added 📈 [gzipped])

name           parsed_size           gzip_size
entry-login         +548 B  (+0.0%)    +4067 B  (+0.6%)
entry-stepper       -200 B  (-0.0%)      -33 B  (-0.0%)
entry-main           +42 B  (+0.0%)      +12 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~13002 bytes added 📈 [gzipped])

name                     parsed_size           gzip_size
site-setup-flow             -17605 B  (-7.6%)    -4455 B  (-7.0%)
checkout                      +174 B  (+0.0%)    +3339 B  (+0.6%)
a8c-for-agencies-client       +174 B  (+0.0%)    +3269 B  (+0.6%)
site-blocks                   +170 B  (+0.0%)    +3339 B  (+2.0%)
security                      +170 B  (+0.0%)    +3339 B  (+1.5%)
purchases                     +170 B  (+0.0%)    +3386 B  (+0.5%)
privacy                       +170 B  (+0.0%)    +3339 B  (+2.1%)
notification-settings         +170 B  (+0.0%)    +3339 B  (+1.6%)
me                            +170 B  (+0.0%)    +3339 B  (+2.1%)
help                          +170 B  (+0.0%)    +3339 B  (+2.0%)
developer                     +170 B  (+0.0%)    +3339 B  (+2.0%)
account-close                 +170 B  (+0.0%)    +3339 B  (+2.0%)
account                       +170 B  (+0.0%)    +3339 B  (+1.6%)
reader                        +148 B  (+0.0%)    +3129 B  (+0.3%)
home                          +145 B  (+0.0%)    +3325 B  (+0.7%)
marketing                      +58 B  (+0.0%)      +16 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~4392 bytes removed 📉 [gzipped])

name                                                         parsed_size           gzip_size
async-load-calypso-blocks-editor-checkout-modal                   +174 B  (+0.0%)    +3339 B  (+1.0%)
async-load-calypso-notifications                                  +168 B  (+0.1%)    +3359 B  (+5.2%)
async-load-design-blocks                                          +145 B  (+0.0%)    +3311 B  (+0.6%)
async-load-calypso-my-sites-site-settings-seo-settings-form       +133 B  (+0.0%)     -226 B  (-0.3%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot · 2025-05-01T14:46:38Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

notifications
wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug remove/client-express-useragent on your sandbox.

tyxla

This is looking good from my perspective!

I imagine you want to get rid of the dependency after landing this one and #103023?

Also going to leave the final review to @jsnajdr in case he has something else to add.

client/lib/user-agent/get-device-type.ts

tyxla · 2025-05-02T11:45:19Z

apps/notifications/src/panel/templates/shortcuts-popover.jsx

@@ -3,7 +3,7 @@ import clsx from 'clsx';
 import { useTranslate } from 'i18n-calypso';
 import { useRef } from 'react';
 import { connect } from 'react-redux';
-import userAgent from 'calypso/lib/user-agent';
+import { isMobile } from 'calypso/lib/user-agent';


Note this will require separate testing and deploying of the notifications app.

See PCYsg-elI-p2 and PCYsg-OT6-p2

Note this will require separate testing and deploying of the notifications app.

@tyxla It's not entirely clear to me from those resources: Is it on me to see through the deployment of that application with these changes, or will it be included in some regular future deployment?

Yep, if you merge a change that touches a specific app, you should also smoke test and deploy that app. That's something that's often overlooked, as we don't currently have a mechanism to nudge or force people to do it.

tyxla · 2025-05-02T11:46:00Z

client/lib/user-agent/get-os-name.ts

@@ -0,0 +1,6 @@
+import { once } from 'lodash';


Same as above wrt lodash.

tyxla · 2025-05-02T11:47:03Z

client/lib/user-agent/test/index.test.ts

+jest.mock( '../get-os-name', () => jest.fn() );
+jest.mock( '../get-device-type', () => jest.fn() );
+
+describe( 'user-agent', () => {


Thanks for the thorough tests!

client/blocks/get-apps/mobile-download-card.tsx

client/lib/user-agent/get-device-type.ts

jsnajdr · 2025-05-02T12:35:38Z

client/landing/stepper/hooks/use-is-site-big-sky-eligible.ts

@@ -25,7 +25,7 @@ export function useIsBigSkyEligible( flowName?: string ) {
 	const { isOwner } = useIsSiteOwner();
 	const site = useSite();
 	const product_slug = site?.plan?.product_slug || '';
-	const onSupportedDevice = userAgent.isTablet || userAgent.isDesktop;
+	const onSupportedDevice = isTablet() || isDesktop();


Our life would be easier if isDesktop didn't exist: because ua-parser-js doesn't support it. For good reasons, the UA string never really says that it's coming from a desktop device.

Looking at #95965 and guessing @dereksmart's intent back then, it seems that ! isMobile() is the right check we want to do.

Looking at the reasons for the isMobile check in #95965:

The UX can now handle narrow viewports, but the mobile keyboard still gets in the way.

it seems that we want to decide according the screen size and avoid screens that are too narrow. For that, we have isMobile check from @automattic/viewport:

import { isMobile } from '@automattic/viewport';

This is used at many places in the Calypso codebase, and the check is based on screen width: <480px is considered mobile. In this case that's a better fit than a UA check.

Updated to use the isMobile viewport helper in b8d8dd5.

Separately, I wonder if it would be clearer to distinguish the name of these, e.g. isMobileViewport and/or isMobileUserAgent.

Separately, I wonder if it would be clearer to distinguish the name of these, e.g. isMobileViewport and/or isMobileUserAgent.

Although I think "mobile viewport" could be a misnomer to begin with, and we should probably refer to viewports as sizes (small, medium, large, etc.).

I see this is handled a bit better with the useViewportMatch hook in @wordpress/compose.

client/login/magic-login/index.jsx

See: #103048 (comment)

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071536519 isMobile() && ! isOS() should not infer that it's Android

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071550870

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071553780

Consolidate client user agent parsing

352493c

aduth requested review from jsnajdr and tyxla May 1, 2025 14:39

aduth requested a review from a team as a code owner May 1, 2025 14:39

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 1, 2025

aduth added the Framework label May 1, 2025

tyxla reviewed May 2, 2025

View reviewed changes

jsnajdr reviewed May 2, 2025

View reviewed changes

aduth added 6 commits May 5, 2025 13:42

Use once utility from memoize-last lib\

509667e

See: #103048 (comment)

Add test coverage for MobileDownloadCard

d6ff898

Update MobileDownloadCard to differ on Android vs. iOS

e51ddc5

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071536519 isMobile() && ! isOS() should not infer that it's Android

Use non-mobile viewport to determine big sky eligible

b8d8dd5

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071550870

Remove unused isDesktop

b15f3c2

Use platform consideration for showing app promo

32b12dd

See: https://github.com/Automattic/wp-calypso/pull/103048/files#r2071553780

aduth mentioned this pull request May 20, 2025

Prototype props table for design system documentation #103561

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate client user agent parsing #103048

Consolidate client user agent parsing #103048

aduth commented May 1, 2025

github-actions bot commented May 1, 2025 •

edited by matticbot

Loading

matticbot commented May 1, 2025 •

edited

Loading

matticbot commented May 1, 2025 •

edited

Loading

tyxla left a comment

tyxla May 2, 2025

aduth May 8, 2025 •

edited

Loading

tyxla May 9, 2025

tyxla May 2, 2025

tyxla May 2, 2025

jsnajdr May 2, 2025

jsnajdr May 5, 2025

aduth May 5, 2025

aduth May 5, 2025

Consolidate client user agent parsing #103048

Are you sure you want to change the base?

Consolidate client user agent parsing #103048

Conversation

aduth commented May 1, 2025

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

github-actions bot commented May 1, 2025 • edited by matticbot Loading

matticbot commented May 1, 2025 • edited Loading

matticbot commented May 1, 2025 • edited Loading

tyxla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth May 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 1, 2025 •

edited by matticbot

Loading

matticbot commented May 1, 2025 •

edited

Loading

matticbot commented May 1, 2025 •

edited

Loading

aduth May 8, 2025 •

edited

Loading