fix: navigator.sendBeacon test (reverted)

[seeratawan01]

PR Type

bug_fix, enhancement

---

Description

• Fixed the condition for setting the transport method in UsermavenClientImpl to ensure it uses the beaconApi property instead of directly checking navigator.sendBeacon.
• Enhanced the initialization logic by adding a check for options.use_beacon_api before enabling the beaconApi.

---

Changes walkthrough 📝

	Relevant files
Enhancement	usermaven.ts Fix and enhance beacon API usage in UsermavenClientImpl    packages/javascript-sdk/src/usermaven.ts Modified the condition for setting the transport method to use beaconApi instead of navigator.sendBeacon. Added a check for options.use_beacon_api before setting beaconApi to true. +2/-4

---

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

[github-actions[bot]]

PR Reviewer Guide 🔍

🏅 Score: 82

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The condition for setting `this.beaconApi` is now dependent on both `navigator.sendBeacon` and `options.use_beacon_api`. This change might lead to unexpected behavior if `options.use_beacon_api` is not explicitly set by the user, potentially breaking existing implementations that rely on the default behavior.

Code feedback:

---

relevant file	packages/javascript-sdk/src/usermaven.ts
suggestion	Consider adding a default value for `options.use_beacon_api` to ensure backward compatibility and prevent potential issues if the property is not explicitly set. This can be achieved by modifying the condition to `if (navigator?.sendBeacon && (options.use_beacon_api !== undefined ? options.use_beacon_api : true))`. This change ensures that `beaconApi` is enabled by default unless explicitly disabled by the user. [important]
relevant line	if (navigator?.sendBeacon && options.use_beacon_api) {

---

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Improve the reliability of feature detection for navigator.sendBeacon ___ **Replace the usage of navigator?.sendBeacon with a more reliable feature detection method. The optional chaining operator (?.) might not be sufficient if navigator is undefined, which can cause runtime errors in non-browser environments.** [packages/javascript-sdk/src/usermaven.ts [975-976]](https://github.com/usermaven/usermaven-js/pull/97/files#diff-a9e765a7bfd2dfbeba110d6b5d12e58d21090506ae52b206581809fae414ebfeR975-R976) ```diff -if (navigator?.sendBeacon && options.use_beacon_api) { +if (typeof navigator !== 'undefined' && navigator.sendBeacon && options.use_beacon_api) { this.beaconApi = true; } ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly addresses a potential runtime error by ensuring `navigator` is defined before accessing `sendBeacon`, which is crucial for environments where `navigator` might be undefined.	9
	Ensure transport methods are initialized before use ___ **Ensure that the beaconTransport and xmlHttpTransport are properly initialized before assignment to this.transport. This avoids potential runtime errors where these transports might not be defined.** [packages/javascript-sdk/src/usermaven.ts [937]](https://github.com/usermaven/usermaven-js/pull/97/files#diff-a9e765a7bfd2dfbeba110d6b5d12e58d21090506ae52b206581809fae414ebfeR937-R937) ```diff -this.transport = this.beaconApi ? beaconTransport : xmlHttpTransport; +if (typeof beaconTransport !== 'undefined' && typeof xmlHttpTransport !== 'undefined') { + this.transport = this.beaconApi ? beaconTransport : xmlHttpTransport; +} else { + throw new Error('Transport methods are not properly initialized.'); +} ``` Suggestion importance[1-10]: 8 Why: The suggestion improves robustness by checking if `beaconTransport` and `xmlHttpTransport` are defined before assignment, preventing potential runtime errors. However, it assumes that these transports might not be initialized, which may not be the case depending on the broader code context.	8