-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: removed unused packages to reduce expected app size by ~1.13 MB
#6892
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: develop
Are you sure you want to change the base?
Conversation
WalkthroughReplaced ua-parser-js with Bowser for user-agent parsing in RoomInfoView and updated package.json to add Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (2)
142-142: Remove the stale resolution forua-parser-js.Since
ua-parser-jsis being removed from the project dependencies, the resolution entry at line 142 should also be removed to avoid confusion and potential issues.♻️ Proposed fix
"resolutions": { - "ua-parser-js": "1.0.2", "jpeg-js": "0.4.4" },
179-179: Remove the stale type definitions forua-parser-js.The type definitions for
ua-parser-jsat line 179 should be removed since the package itself is being removed.Additionally, verify whether Bowser requires separate type definitions or if it includes them natively:
#!/bin/bash # Description: Check if Bowser includes TypeScript definitions or needs @types/bowser # Check if bowser package includes types curl -s https://registry.npmjs.org/bowser/2.13.1 | jq -r '.types // .typings // "No types found in package"' # Check if @types/bowser exists curl -s https://registry.npmjs.org/@types/bowser 2>&1 | grep -q '"error":"Not found"' && echo "@types/bowser does not exist" || echo "@types/bowser exists"♻️ Proposed fix to remove stale types
"@types/semver": "7.3.13", - "@types/ua-parser-js": "^0.7.36", "@types/url-parse": "^1.4.8",
🤖 Fix all issues with AI agents
In @app/views/RoomInfoView/index.tsx:
- Around line 134-136: The OS/browser strings set from
Bowser.getParser(visitor.userAgent) can include undefined version fields,
leading to outputs like "Windows undefined"; update the assignments that set
params.os and params.browser to defensively handle missing name or version by
reading ua.getOS() and ua.getBrowser(), defaulting name/version to empty string
or omitting the version when undefined, and composing the final string
accordingly (e.g. use name only if version is falsy or join name and version
with a conditional). Target the code around Bowser.getParser, visitor.userAgent,
params.os and params.browser and ensure the final values never contain the
literal "undefined".
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
app/views/RoomInfoView/index.tsxpackage.json
🔇 Additional comments (2)
app/views/RoomInfoView/index.tsx (1)
8-8: The Bowser import is correct and compatible with v2.13.1.Bowser.getParser(), Parser.getOS(), and Parser.getBrowser() are all supported in Bowser v2.13.1 with the correct return types matching the usage pattern in this file.
package.json (1)
54-54: Bowser version 2.13.1 exists on npm and has no known security vulnerabilities. The package is legitimate and actively maintained. No actionable changes needed.
~1.13 MB~1.13 MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (2)
142-143: Remove obsolete resolution entry for ua-parser-js.Since ua-parser-js has been removed from dependencies, the resolution entry at line 142 is no longer needed and should be removed.
♻️ Proposed fix
"resolutions": { - "ua-parser-js": "1.0.2", "jpeg-js": "0.4.4" },
179-179: Remove obsolete type definitions for ua-parser-js.Since ua-parser-js has been removed from dependencies, the type definitions
@types/ua-parser-jsin devDependencies are no longer needed.♻️ Proposed fix
"@types/semver": "7.3.13", - "@types/ua-parser-js": "^0.7.36", "@types/url-parse": "^1.4.8",
🤖 Fix all issues with AI agents
In @app/views/RoomInfoView/index.tsx:
- Around line 134-138: The code uses Bowser.getParser and then falls back to
convenience methods (ua.getOSName()/getBrowserName()) which can return the
literal string "undefined"; replace those fallbacks by only using the parsed
objects (ua.getOS().name and ua.getOS().version; ua.getBrowser().name and
ua.getBrowser().version), coalescing each to an empty string and building
params.os and params.browser from those values, then call .trim() on the final
string to avoid stray spaces (update references around Bowser.getParser,
ua.getOS, ua.getBrowser, params.os and params.browser).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/views/RoomInfoView/index.tsxpackage.json
🔇 Additional comments (2)
app/views/RoomInfoView/index.tsx (1)
8-8: LGTM!The import change from ua-parser-js to Bowser is correct and aligns with the PR objective to reduce bundle size.
package.json (1)
54-54: Verified: ua-parser-js and prop-types have been completely removed from the codebase. The addition of Bowser as a replacement is safe to proceed with.
| const ua = Bowser.getParser(visitor.userAgent); | ||
| params.os = `${ua.getOS().name ?? ua.getOSName()} ${ua.getOS().version ?? ua.getOSVersion()}`; | ||
| params.browser = `${ua.getBrowser().name ?? ua.getBrowserName()} ${ | ||
| ua.getBrowser().version ?? ua.getBrowserVersion() | ||
| }`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Bowser v2.13.1 getOS() getBrowser() return undefined properties API documentation
💡 Result:
Short answer: In Bowser 2.x (including v2.13.1) getBrowser()/getOS() are instance methods on a Parser returned by Bowser.getParser(...) (or new Parser(ua)), not top-level static functions. Calling Bowser.getBrowser() / Bowser.getOS() (or calling instance methods before parsing or with an empty UA) will yield undefined. See docs and parser API.
What to do
- Use the Parser API:
const parser = Bowser.getParser(navigator.userAgent);
const browser = parser.getBrowser(); // { name?, version? }
const os = parser.getOS(); // { name?, version?, versionName? }
or use parser.getResult() for the full parsed object. - Ensure you pass a non-empty UA and that parsing was not skipped (skipParsing=false) — otherwise properties can be undefined.
Sources
- Bowser docs / Parser API. [1]
- parser.js reference showing getBrowser/getOS return possibly undefined name/version fields. [2]
Sources:
[1] https://bowser-js.github.io/bowser/
[2] https://bowser-js.github.io/bowser/docs/parser.js.html
🌐 Web query:
Bowser getOSName() getBrowserName() getOSVersion() getBrowserVersion() methods API
💡 Result:
Short answer: These are methods on Bowser's Parser object. Call Bowser.getParser(userAgent) (or Bowser.getParser(window.navigator.userAgent)), then use getBrowserName()/getBrowserVersion() to get the browser name and version, and getOSName()/getOSVersion() (or getOS()) to get OS name/version. Example:
- const parser = Bowser.getParser(window.navigator.userAgent); [1]
- parser.getBrowserName(); // returns browser name string (e.g., "Chrome"). [1][2]
- parser.getBrowserVersion(); // returns browser version string (e.g., "116.0.5845.96"). [2]
- parser.getOSName(); // returns OS name string (e.g., "Windows" or "macOS"). [2]
- parser.getOSVersion(); // returns OS version string (e.g., "10.15.7"). [2]
- parser.getOS(); // returns { name: "...", version: "..." } if you need both. [2]
Notes: methods live on the Parser instance (Bowser.getParser(...)). See Bowser docs and the Parser API for details and examples. [1][2]
Sources:
[1] Bowser docs (overview / usage).
[2] Bowser Parser API (getBrowserName, getBrowserVersion, getOSName, getOSVersion, getOS).
🌐 Web query:
Bowser v2.13.1 parser getOS() undefined properties when name version not available
💡 Result:
Summary: In Bowser v2.13.x the Parser.getOS() can return an empty object ({} ) when no OS parser matches, and Parser.getOSName() converts the possibly-undefined name with String(name).toLowerCase() — which produces the literal string "undefined" instead of an empty string. This explains why you see undefined properties / the string "undefined" when name/version aren't available. (bowser-js.github.io)
Fix/workarounds:
- Short-term (call-site): read the raw object and null-coalesce: const os = parser.getOS(); const osName = os.name ?? ''; const osVersion = os.version ?? '';
- Short-term (use API): avoid getOSName() in affected versions; use parser.getOS().name with null coalescing. (bowser-js.github.io)
- Long-term (library patch): change getOSName implementation to guard before String(), e.g.:
getOSName(toLowerCase){ const name = this.getOS().name || ''; return toLowerCase ? name.toLowerCase() : name; }
(Or wrap with String(name || '')). This prevents "undefined". (bowser-js.github.io)
If you want, I can draft a small PR patch or a one-line patch you can apply to v2.13.1.
Citations:
- 1: https://bowser-js.github.io/bowser/docs/parser.js.html?utm_source=openai
- 2: https://bowser-js.github.io/bowser/docs/parser.js.html?utm_source=openai
- 3: https://bowser-js.github.io/bowser/docs/parser.js.html?utm_source=openai
Avoid using Bowser's convenience methods as fallbacks; they have a bug that produces the string "undefined".
The current fallback pattern (ua.getOS().name ?? ua.getOSName()) attempts to handle cases where getOS().name is undefined, but Bowser v2.13.1's convenience methods (getOSName(), getBrowserName(), etc.) have a bug: when the properties are unavailable, they return the literal string "undefined" instead of an empty string. This causes params.os and params.browser to contain invalid values like "undefined 116.0".
Instead, use null coalescing with empty strings:
params.os = `${ua.getOS().name ?? ''} ${ua.getOS().version ?? ''}`.trim();
params.browser = `${ua.getBrowser().name ?? ''} ${ua.getBrowser().version ?? ''}`.trim();Or apply .trim() to the final string to clean up extra spaces if either property is missing.
🤖 Prompt for AI Agents
In @app/views/RoomInfoView/index.tsx around lines 134 - 138, The code uses
Bowser.getParser and then falls back to convenience methods
(ua.getOSName()/getBrowserName()) which can return the literal string
"undefined"; replace those fallbacks by only using the parsed objects
(ua.getOS().name and ua.getOS().version; ua.getBrowser().name and
ua.getBrowser().version), coalescing each to an empty string and building
params.os and params.browser from those values, then call .trim() on the final
string to avoid stray spaces (update references around Bowser.getParser,
ua.getOS, ua.getBrowser, params.os and params.browser).
Proposed changes
This PR reduces the app bundle size by removing unused / heavyweight dependencies and replacing them with a lighter, functionally equivalent alternative.
Summary
(~94.5 kB)(~1.29 MB)Added a lightweight alternative:
(~252 kB)The only usage of
ua-parser-jswas inRoomInfoView(Livechat visitor OS & browser parsing).This has been replaced with
Bowser, which provides the same required information (OS + browser) with a significantly smaller footprint and no functional impact.Why this change?
ua-parser-jsis a large dependency for a single, limited use casebowserprovides equivalent parsing for OS and browser while being:Issue(s)
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Screenshots
Attached screenshots show before and after behavior for the Room Info view to demonstrate no visual or functional regressions.
Summary by CodeRabbit
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.