Skip to content

fix(isMobile): performance #646

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.

Sign up for GitHub

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

Merged
zombieJ merged 1 commit into from
Jun 16, 2025
Merged

fix(isMobile): performance #646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
]
},
"dependencies": {
"is-mobile": "^5.0.0",
"react-is": "^18.2.0"
},
"devDependencies": {
Expand Down
19 changes: 7 additions & 12 deletions src/isMobile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import isMobile from 'is-mobile';
Copy link

@coderabbitai coderabbitai bot Jun 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

CommonJS 默认导出可能导致类型/运行时问题

is-mobile 的类型声明采用 export = isMobile，在未开启 esModuleInterop 时应使用

import isMobile = require('is-mobile');

或

import * as isMobile from 'is-mobile';

否则编译阶段会出现 TS1192，运行时也可能拿到 { default: [Function] } 而非函数。

🤖 Prompt for AI Agents 
In src/isMobile.ts at line 1, the import statement uses ES module default import
syntax for a CommonJS module, which can cause TypeScript error TS1192 and
runtime issues. Replace the current import with either "import isMobile =
require('is-mobile');" or "import * as isMobile from 'is-mobile';" to correctly
import the CommonJS default export without enabling esModuleInterop.


let cached: boolean;

Comment on lines +3 to +4
Copy link

@coderabbitai coderabbitai bot Jun 16, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

缓存变量作用域过宽，SSR 场景下存在误判风险

cached 是模块级静态变量。在 Node SSR 中，多个 HTTP 请求会共享同一模块实例：
首个移动端请求若将 cached 置为 true，后续桌面请求也会错误地获得 true。

建议：

-export default () => {
-  if (typeof cached === 'undefined') {
-    cached = isMobile();
-  }
-  return cached;
-};
+export default (ua?: string) => {
+  // 若显式传入 UA，不走全局缓存
+  if (ua) {
+    return isMobile({ ua });
+  }
+  if (typeof cached === 'undefined') {
+    cached = isMobile();
+  }
+  return cached;
+};

这样浏览端依旧命中缓存，而服务端可按请求 UA 调用。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let cached: boolean;
let cached: boolean;
export default (ua?: string) => {
// 若显式传入 UA，不走全局缓存
if (ua) {
return isMobile({ ua });
}
if (typeof cached === 'undefined') {
cached = isMobile();
}
return cached;
};
🤖 Prompt for AI Agents 
In src/isMobile.ts at lines 3 to 4, the cached variable is declared at the
module level, causing shared state across requests in SSR environments and
leading to incorrect mobile detection. To fix this, remove the module-level
cached variable and instead implement caching scoped per request or per function
call, ensuring that server-side rendering does not share cached results between
different user agents while still allowing caching on the client side.

export default () => {
if (typeof navigator === 'undefined' || typeof window === 'undefined') {
return false;
if (typeof cached === 'undefined') {
cached = isMobile();
}

const agent =
navigator.userAgent || navigator.vendor || (window as any).opera;
return (
/(android|bb\d+|meego).+mobile|avantgo|bada\/|blackberry|blazer|compal|elaine|fennec|hiptop|iemobile|ip(hone|od)|iris|kindle|lge |maemo|midp|mmp|mobile.+firefox|netfront|opera m(ob|in)i|palm( os)?|phone|p(ixi|re)\/|plucker|pocket|psp|series(4|6)0|symbian|treo|up\.(browser|link)|vodafone|wap|windows ce|xda|xiino|android|ipad|playbook|silk/i.test(
agent,
) ||
/1207|6310|6590|3gso|4thp|50[1-6]i|770s|802s|a wa|abac|ac(er|oo|s-)|ai(ko|rn)|al(av|ca|co)|amoi|an(ex|ny|yw)|aptu|ar(ch|go)|as(te|us)|attw|au(di|-m|r |s )|avan|be(ck|ll|nq)|bi(lb|rd)|bl(ac|az)|br(e|v)w|bumb|bw-(n|u)|c55\/|capi|ccwa|cdm-|cell|chtm|cldc|cmd-|co(mp|nd)|craw|da(it|ll|ng)|dbte|dc-s|devi|dica|dmob|do(c|p)o|ds(12|-d)|el(49|ai)|em(l2|ul)|er(ic|k0)|esl8|ez([4-7]0|os|wa|ze)|fetc|fly(-|_)|g1 u|g560|gene|gf-5|g-mo|go(\.w|od)|gr(ad|un)|haie|hcit|hd-(m|p|t)|hei-|hi(pt|ta)|hp( i|ip)|hs-c|ht(c(-| |_|a|g|p|s|t)|tp)|hu(aw|tc)|i-(20|go|ma)|i230|iac( |-|\/)|ibro|idea|ig01|ikom|im1k|inno|ipaq|iris|ja(t|v)a|jbro|jemu|jigs|kddi|keji|kgt( |\/)|klon|kpt |kwc-|kyo(c|k)|le(no|xi)|lg( g|\/(k|l|u)|50|54|-[a-w])|libw|lynx|m1-w|m3ga|m50\/|ma(te|ui|xo)|mc(01|21|ca)|m-cr|me(rc|ri)|mi(o8|oa|ts)|mmef|mo(01|02|bi|de|do|t(-| |o|v)|zz)|mt(50|p1|v )|mwbp|mywa|n10[0-2]|n20[2-3]|n30(0|2)|n50(0|2|5)|n7(0(0|1)|10)|ne((c|m)-|on|tf|wf|wg|wt)|nok(6|i)|nzph|o2im|op(ti|wv)|oran|owg1|p800|pan(a|d|t)|pdxg|pg(13|-([1-8]|c))|phil|pire|pl(ay|uc)|pn-2|po(ck|rt|se)|prox|psio|pt-g|qa-a|qc(07|12|21|32|60|-[2-7]|i-)|qtek|r380|r600|raks|rim9|ro(ve|zo)|s55\/|sa(ge|ma|mm|ms|ny|va)|sc(01|h-|oo|p-)|sdk\/|se(c(-|0|1)|47|mc|nd|ri)|sgh-|shar|sie(-|m)|sk-0|sl(45|id)|sm(al|ar|b3|it|t5)|so(ft|ny)|sp(01|h-|v-|v )|sy(01|mb)|t2(18|50)|t6(00|10|18)|ta(gt|lk)|tcl-|tdg-|tel(i|m)|tim-|t-mo|to(pl|sh)|ts(70|m-|m3|m5)|tx-9|up(\.b|g1|si)|utst|v400|v750|veri|vi(rg|te)|vk(40|5[0-3]|-v)|vm40|voda|vulc|vx(52|53|60|61|70|80|81|83|85|98)|w3c(-| )|webc|whit|wi(g |nc|nw)|wmlb|wonu|x700|yas-|your|zeto|zte-/i.test(
agent?.substr(0, 4),
)
);
return cached;
};
8 changes: 0 additions & 8 deletions tests/hooks.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,14 +515,6 @@ describe('hooks', () => {

const { container } = render(<Demo />);
expect(container.textContent).toBe('pc');

const navigatorSpy = jest
.spyOn(navigator, 'userAgent', 'get')
.mockImplementation(() => 'Android');
const { container: container2 } = render(<Demo />);
expect(container2.textContent).toBe('mobile');

navigatorSpy.mockRestore();
});

it('should not warn useLayoutEffect in SSR', () => {
Expand Down
Loading