fix: navigator.sendBeacon test

seeratawan01 commented 1 month ago

bug_fix

Fixed a bug in the UsermavenClientImpl class where the condition for selecting the transport method was incorrectly checking this.beaconApi instead of navigator.sendBeacon.
Improved safety by using a nullish coalescing operator when accessing navigator.

Relevant files

Bug fix

usermaven.ts

Fix condition for selecting transport method in UsermavenClient

packages/javascript-sdk/src/usermaven.ts

Updated the condition to check for navigator.sendBeacon instead of
this.beaconApi.

Added a nullish coalescing operator to ensure safe access to
navigator.

+3/-1

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

🏅 Score: 92

🧪 No relevant tests

🔒 No security concerns identified

⚡ No key issues to review

Code feedback:

relevant file	packages/javascript-sdk/src/usermaven.ts
suggestion	Consider adding a fallback or error handling for cases where `navigator` is undefined to prevent runtime errors in non-browser environments. [important]
relevant line	this.transport = navigator?.sendBeacon ? beaconTransport : xmlHttpTransport;

github-actions[bot] commented 1 month ago

Category	Suggestion	Score
Best practice	Remove the optional chaining operator to simplify the code ___ Replace the optional chaining operator (`?.`) with a direct property access for `navigator.sendBeacon` since the `isWindowAvailable()` check already ensures that the window object is available, making the optional chaining redundant. [packages/javascript-sdk/src/usermaven.ts [938]](https://github.com/usermaven/usermaven-js/pull/96/files#diff-a9e765a7bfd2dfbeba110d6b5d12e58d21090506ae52b206581809fae414ebfeR938-R938) ```diff -this.transport = navigator?.sendBeacon ? beaconTransport : xmlHttpTransport; +this.transport = navigator.sendBeacon ? beaconTransport : xmlHttpTransport; ``` Suggestion importance[1-10]: 9 Why: The suggestion is correct and improves code readability by removing unnecessary optional chaining. The `isWindowAvailable()` check ensures that the window object is available, making the optional chaining redundant. This is a good practice for cleaner and more efficient code.	9