Skip to content
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

chore: fix globalThis polyfill in loading snippet #1821

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

saikumarrs
Copy link
Member

@saikumarrs saikumarrs commented Aug 5, 2024

PR Description

Fixed the custom globalThis polyfill in the loading snippet.

Linear task (optional)

https://linear.app/rudderstack/issue/SDK-2239/update-globalthis-polyfill-for-safari-browsers

Cross Browser Tests

Please confirm you have tested for the following browsers:

  • Chrome
  • Safari
  • IE11

Sanity Suite

  • All sanity suite test cases pass locally

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

Summary by CodeRabbit

  • Documentation

    • Streamlined integration instructions in the README by removing lengthy code snippets and providing a concise reference to the official documentation for the JavaScript SDK.
  • New Features

    • Enhanced the logic for defining globalThis across various JavaScript files, ensuring safer, more robust global context assignments without modifying Object.prototype. This improves compatibility and reduces potential side effects.

These changes collectively enhance user experience by providing clearer documentation and a more reliable integration method for developers.

@saikumarrs saikumarrs self-assigned this Aug 5, 2024
@saikumarrs saikumarrs requested a review from a team as a code owner August 5, 2024 09:45
Copy link
Contributor

coderabbitai bot commented Aug 5, 2024

Walkthrough

Walkthrough

The recent changes focus on enhancing the definition of the globalThis context across various JavaScript files. The previous approach of modifying Object.prototype has been replaced with a more robust mechanism using immediately invoked function expressions (IIFEs). This change ensures safer checks for the global object across different environments while preventing prototype pollution, resulting in cleaner and more maintainable code.

Changes

File(s) Change Summary
README.md Removed lengthy JavaScript SDK integration code snippet; added a concise link to official documentation for CDN installation.
packages/analytics-js/public/index.html, packages/sanity-suite/public/v3/*.html Replaced direct definition of globalThis on Object.prototype with IIFE that checks for the global context (self, window). Improved encapsulation and safety.
packages/loading-scripts/src/index.ts Updated global polyfill for globalThis using IIFE to determine the appropriate global context without modifying Object.prototype, enhancing robustness and safety.
packages/sanity-suite/public/v3/manualLoadCall/*.html Similar updates as above, enhancing global context definition through IIFE without prototype modification, ensuring safer and clearer assignments of globalThis.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SDK
    participant GlobalContext

    User->>SDK: Load SDK
    SDK->>GlobalContext: Check for `globalThis`
    alt `globalThis` is undefined
        GlobalContext->>GlobalContext: Define `getGlobal`
        GlobalContext->>GlobalContext: Check for `self` or `window`
        GlobalContext-->>SDK: Return valid global object
        SDK->>GlobalContext: Assign `globalThis`
    else `globalThis` exists
        SDK-->>User: Continue as normal
    end
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.70%. Comparing base (e63bbe6) to head (38d75ec).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1821   +/-   ##
========================================
  Coverage    56.70%   56.70%           
========================================
  Files          473      473           
  Lines        16121    16121           
  Branches      3229     3226    -3     
========================================
  Hits          9141     9141           
- Misses        5713     5736   +23     
+ Partials      1267     1244   -23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e63bbe6 and 389ad8f.

Files selected for processing (15)
  • README.md (1 hunks)
  • packages/analytics-js/public/index.html (1 hunks)
  • packages/loading-scripts/src/index.ts (1 hunks)
  • packages/sanity-suite/public/v3/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/index-npm.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-npm.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm.html (1 hunks)
Files skipped from review due to trivial changes (1)
  • README.md
Additional context used
Learnings (2)
packages/sanity-suite/public/v3/index-cdn.html (1)
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-common/src/utilities/page.ts:1-34
Timestamp: 2024-06-14T06:40:08.622Z
Learning: The compatibility issue with `globalThis` in `packages/analytics-js-common/src/utilities/page.ts` is handled elsewhere in the codebase as per user `saikumarrs`.
packages/analytics-js/public/index.html (1)
Learnt from: saikumarrs
PR: rudderlabs/rudder-sdk-js#1754
File: packages/analytics-js-common/src/utilities/page.ts:1-34
Timestamp: 2024-06-14T06:40:08.622Z
Learning: The compatibility issue with `globalThis` in `packages/analytics-js-common/src/utilities/page.ts` is handled elsewhere in the codebase as per user `saikumarrs`.
Additional comments not posted (14)
packages/loading-scripts/src/index.ts (1)

102-123: LGTM! Robust implementation of globalThis polyfill.

The use of an IIFE and the getGlobal function ensures that the global object is correctly determined without modifying Object.prototype. This approach is safer and more maintainable.

packages/sanity-suite/public/v3/index-npm-bundled.html (1)

98-119: LGTM! Robust implementation of globalThis polyfill.

The use of an IIFE and the getGlobal function ensures that the global object is correctly determined without modifying Object.prototype. This approach is safer and more maintainable.

packages/sanity-suite/public/v3/index-npm.html (1)

98-119: LGTM! Robust implementation of globalThis polyfill.

The use of an IIFE and the getGlobal function ensures that the global object is correctly determined without modifying Object.prototype. This approach is safer and more maintainable.

packages/sanity-suite/public/v3/integrations/index-npm-bundled.html (1)

84-105: LGTM! Improved globalThis polyfill implementation.

The new implementation enhances safety and clarity by avoiding prototype pollution and ensuring that globalThis is only defined if a valid global object is found.

packages/sanity-suite/public/v3/integrations/index-npm.html (1)

84-105: LGTM! Improved globalThis polyfill implementation.

The new implementation enhances safety and clarity by avoiding prototype pollution and ensuring that globalThis is only defined if a valid global object is found.

packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html (1)

84-105: LGTM! Improved globalThis polyfill implementation.

The new implementation enhances safety and clarity by avoiding prototype pollution and ensuring that globalThis is only defined if a valid global object is found.

packages/sanity-suite/public/v3/manualLoadCall/index-npm.html (1)

84-104: LGTM! Improved encapsulation and safety for polyfilling globalThis.

The new implementation avoids modifying Object.prototype and uses a helper function to determine the appropriate global object. This approach enhances robustness and maintainability.

However, ensure that all instances of globalThis usage are correctly handled across the codebase.

Verification successful

Let's re-run the search for globalThis using the correct file type for JavaScript files and include HTML files as well.


LGTM! Improved encapsulation and safety for polyfilling globalThis.

The new implementation avoids modifying Object.prototype and uses a helper function to determine the appropriate global object. This approach enhances robustness and maintainability. The search results confirm that the new polyfill method is consistently applied across the codebase, and there are no instances of the old polyfill method.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `globalThis` usage in the codebase.

# Test: Search for the usage of `globalThis`. Expect: No instances of the old polyfill method.
rg --type javascript --type html -A 5 $'globalThis'

Length of output: 85


Script:

#!/bin/bash
# Description: Verify all instances of `globalThis` usage in the codebase.

# Test: Search for the usage of `globalThis`. Expect: No instances of the old polyfill method.
rg --type js --type html -A 5 $'globalThis'

Length of output: 349181

packages/sanity-suite/public/v3/index-local.html (1)

84-104: LGTM! Improved encapsulation and safety for defining globalThis.

The new implementation avoids modifying Object.prototype and uses a helper function to determine the appropriate global object. This approach enhances robustness and maintainability.

However, ensure that all instances of globalThis usage are correctly handled across the codebase.

Verification successful

Verification successful!

The new polyfill method for globalThis is consistently used across the codebase, and no instances of the old polyfill method were found. This ensures that the usage of globalThis is correctly handled.

  • packages/sanity-suite/src/testBook/string.js
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html
  • packages/sanity-suite/public/v3/integrations/index-npm.html
  • packages/sanity-suite/public/v3/integrations/index-npm-bundled.html
  • packages/sanity-suite/public/v3/integrations/index-cdn.html
  • packages/sanity-suite/public/v3/index-npm.html
  • packages/sanity-suite/public/v3/index-local.html
  • packages/sanity-suite/public/v3/integrations/index-local.html
  • packages/sanity-suite/public/v3/index-npm-bundled.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm.html
  • packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html
  • packages/sanity-suite/public/v1.1/integrations/index-npm.html
  • packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html
  • packages/sanity-suite/public/v1.1/integrations/index-local.html
  • packages/sanity-suite/public/v1.1/manualLoadCall/index-local.html
  • packages/sanity-suite/public/v1.1/integrations/index-cdn.html
  • packages/sanity-suite/public/v1.1/index-local.html
  • packages/sanity-suite/public/v1.1/index-cdn.html
  • packages/analytics-js/__tests__/nativeSdkLoader.js
  • jest/jest.setup-dom.js
  • packages/analytics-js/public/index.html
  • examples/v3-beacon/index.html
  • packages/analytics-js-common/src/v1.1/utils/storage/v3DecryptionUtils.js
  • examples/v3-legacy-minimum-plugins/index.html
  • examples/v3/index.html
  • examples/v3-minimum-plugins/index.html
  • examples/v3-legacy/index.html
  • examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/public/index.html
  • examples/chrome-extension/content-script-v3/foreground.js
  • examples/chrome-extension/background-script/rudderAnalytics.js
  • examples/chrome-extension/background-script-websockets/rudderAnalytics.js
  • assets/integrations/AdobeAnalytics/adobe-analytics-js.js
  • assets/integrations/AdobeAnalytics/adobe-analytics-js-heartbeat.js
  • examples/chrome-extension/content-script-v1.1/foreground.js
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `globalThis` usage in the codebase.

# Test: Search for the usage of `globalThis`. Expect: No instances of the old polyfill method.
rg --type javascript --type html -A 5 $'globalThis'

Length of output: 85


Script:

#!/bin/bash
# Description: Verify all instances of `globalThis` usage in the codebase.

# Find all JavaScript and HTML files and search for `globalThis` usage within them.
fd -e js -e html | xargs rg 'globalThis'

Length of output: 229788

packages/sanity-suite/public/v3/index-cdn.html (1)

84-104: LGTM! Improved encapsulation and safety for defining globalThis.

The new implementation avoids modifying Object.prototype and uses a helper function to determine the appropriate global object. This approach enhances robustness and maintainability.

However, ensure that all instances of globalThis usage are correctly handled across the codebase.

packages/sanity-suite/public/v3/integrations/index-cdn.html (1)

84-104: LGTM! The globalThis polyfill is implemented safely.

The new implementation avoids prototype pollution and enhances safety by using an IIFE and a helper function getGlobal.

However, ensure that all references to globalThis in the codebase are correctly handled.

Verification successful

All references to globalThis are correctly handled in the codebase.

The references are used appropriately in various contexts such as setting properties, accessing properties, and using it in functions.

  • Files and Lines:
    • assets/integrations/AdobeAnalytics/adobe-analytics-js.js
    • assets/integrations/AdobeAnalytics/adobe-analytics-js-heartbeat.js
    • packages/sanity-suite/src/testBook/string.js
    • examples/chrome-extension/content-script-v3/foreground.js
    • examples/chrome-extension/background-script/rudderAnalytics.js
    • examples/chrome-extension/background-script-websockets/rudderAnalytics.js
    • jest/jest.setup-dom.js
    • examples/chrome-extension/content-script-v1.1/foreground.js
    • packages/analytics-js-common/src/v1.1/utils/storage/v3DecryptionUtils.js
    • packages/analytics-js/__tests__/nativeSdkLoader.js
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `globalThis` in the codebase.

# Test: Search for the usage of `globalThis`. Expect: Correct handling of `globalThis`.
rg --type javascript -A 5 $'globalThis'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all references to `globalThis` in the codebase.

# Test: Search for the usage of `globalThis`. Expect: Correct handling of `globalThis`.
rg --type js -A 5 $'globalThis'

Length of output: 309334

packages/sanity-suite/public/v3/integrations/index-local.html (1)

84-104: LGTM! The globalThis polyfill is implemented safely.

The new implementation avoids prototype pollution and enhances safety by using an IIFE and a helper function getGlobal.

However, ensure that all references to globalThis in the codebase are correctly handled.

Verification successful

All references to globalThis are correctly handled.

The search results indicate that globalThis is used consistently across various files and contexts, and there are no apparent issues with its usage.

  • Files and lines where globalThis is used:

    • assets/integrations/AdobeAnalytics/adobe-analytics-js.js
    • assets/integrations/AdobeAnalytics/adobe-analytics-js-heartbeat.js
    • packages/analytics-js/__tests__/nativeSdkLoader.js
    • packages/loading-scripts/src/index.ts
    • packages/analytics-js/public/index.html
    • packages/analytics-js/src/app/RudderAnalytics.ts
    • packages/analytics-js/src/components/preloadBuffer/index.ts
    • packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
    • packages/analytics-js/src/components/utilities/page.ts
    • packages/analytics-js/src/components/capabilitiesManager/CapabilitiesManager.ts
    • packages/analytics-js/src/components/utilities/globals.ts
    • packages/analytics-js/src/components/core/Analytics.ts
    • packages/analytics-js/src/components/core/utilities.ts
    • packages/analytics-js/src/components/eventManager/utilities.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/browser.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/screen.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/storage.ts
    • packages/analytics-js/src/services/ErrorHandler/ErrorHandler.ts
    • packages/analytics-js/src/services/StoreManager/top-domain/index.ts
    • packages/analytics-js/src/services/StoreManager/storages/defaultOptions.ts
    • packages/analytics-js/src/services/StoreManager/storages/sessionStorage.ts
    • packages/analytics-js/src/components/eventRepository/EventRepository.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/dom.ts
    • packages/analytics-js/src/state/slices/context.ts
    • packages/sanity-suite/src/testBook/string.js
    • packages/sanity-suite/public/v1.1/index-cdn.html
    • packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html
    • packages/sanity-suite/public/v3/index-npm.html
    • packages/sanity-suite/public/v3/index-cdn.html
    • packages/sanity-suite/public/v3/integrations/index-local.html
    • packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html
    • packages/sanity-suite/public/v3/integrations/index-cdn.html
    • packages/sanity-suite/public/v3/index-local.html
    • packages/sanity-suite/public/v1.1/index-local.html
    • packages/analytics-js-plugins/src/utilities/retryQueue/RetryQueue.ts
    • packages/analytics-js-plugins/src/utilities/retryQueue/Schedule.ts
    • packages/analytics-js-plugins/src/oneTrustConsentManager/index.ts
    • packages/analytics-js-plugins/src/googleLinker/base64Decoder.ts
    • packages/analytics-js-plugins/src/ketchConsentManager/index.ts
    • packages/analytics-js-plugins/src/bugsnag/utils.ts
    • packages/analytics-js-plugins/src/errorReporting/utils.ts
    • packages/analytics-js-plugins/src/deviceModeDestinations/utils.ts
    • packages/analytics-js-common/src/utilities/url.ts
    • packages/analytics-js-common/src/utilities/string.ts
    • packages/analytics-js-common/src/utilities/page.ts
    • packages/analytics-js-common/src/utilities/crypto.ts
    • packages/analytics-js-cookies/src/component-cookie/index.ts
    • packages/analytics-js-common/src/services/ExternalSrcLoader/jsFileLoader.ts
    • packages/analytics-js-common/src/v1.1/utils/storage/v3DecryptionUtils.js
    • packages/analytics-js-cookies/__tests__/cookieUtilities.test.ts
    • examples/v3/index.html
    • examples/v3-minimum-plugins/index.html
    • examples/v3-legacy/index.html
    • examples/v3-beacon/index.html
    • examples/v3-legacy-minimum-plugins/index.html
    • examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/public/index.html
    • examples/chrome-extension/content-script-v3/foreground.js
    • examples/chrome-extension/background-script-websockets/rudderAnalytics.js
    • examples/chrome-extension/background-script/rudderAnalytics.js
    • examples/chrome-extension/content-script-v1.1/foreground.js
  • Category:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `globalThis` in the codebase.

# Test: Search for the usage of `globalThis`. Expect: Correct handling of `globalThis`.
rg --type javascript -A 5 $'globalThis'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all references to `globalThis` in the codebase.

# Test: Search for the usage of `globalThis`. Expect: Correct handling of `globalThis`.
rg --type js --type ts --type html -A 5 $'globalThis'

Length of output: 402702

packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html (1)

84-104: LGTM! The globalThis polyfill is implemented safely.

The new implementation avoids prototype pollution and enhances safety by using an IIFE and a helper function getGlobal.

However, ensure that all references to globalThis in the codebase are correctly handled.

packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1)

84-104: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to window.rudderAnalyticsMount are correctly updated and tested.

Verification successful

Verified: Function usage is consistent and correctly updated.

The function window.rudderAnalyticsMount is consistently defined and invoked across multiple HTML files. The polyfill for globalThis is included in all instances, and there are no discrepancies in its usage.

  • packages/sanity-suite/public/v3/index-cdn.html
  • packages/sanity-suite/public/v3/index-local.html
  • packages/sanity-suite/public/v3/integrations/index-local.html
  • packages/sanity-suite/public/v3/integrations/index-cdn.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-local.html
  • examples/v3/index.html
  • examples/v3-minimum-plugins/index.html
  • packages/analytics-js/public/index.html
  • examples/v3-legacy/index.html
  • examples/v3-legacy-minimum-plugins/index.html
  • examples/v3-beacon/index.html
  • examples/integrations/Ninetailed/sample-apps/app-using-v3-cdn/public/index.html
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `window.rudderAnalyticsMount` are correctly updated and tested.

# Test: Search for the function usage. Expect: Only occurrences of the new implementation.
rg --type html -A 5 $'window.rudderAnalyticsMount'

Length of output: 14597

packages/analytics-js/public/index.html (1)

62-83: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to window.rudderAnalyticsMount are correctly updated and tested.

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 5, 2024
Copy link

github-actions bot commented Aug 5, 2024

size-limit report 📦

Name Size (Base) Size (Current) Size Limit Status
Cookies Utils - Legacy - NPM (ESM) 1.54 KB 1.54 KB (0%) 2 KB
Cookies Utils - Legacy - NPM (CJS) 1.75 KB 1.75 KB (0%) 2 KB
Cookies Utils - Legacy - NPM (UMD) 1.53 KB 1.53 KB (0%) 2 KB
Cookies Utils - Modern - NPM (ESM) 1.17 KB 1.17 KB (0%) 1.5 KB
Cookies Utils - Modern - NPM (CJS) 1.4 KB 1.4 KB (0%) 1.5 KB
Cookies Utils - Modern - NPM (UMD) 1.16 KB 1.16 KB (0%) 1.5 KB
Plugins Module Federation Mapping - Legacy - CDN 332 B 332 B (0%) 512 B
Plugins - Legacy - CDN 15.88 KB 15.88 KB (0%) 16 KB
Plugins Module Federation Mapping - Modern - CDN 331 B 331 B (0%) 512 B
Plugins - Modern - CDN 7.19 KB 7.19 KB (0%) 7.5 KB
Common - No bundling 15.89 KB 15.89 KB (0%) 16.5 KB
Service Worker - Legacy - NPM (ESM) 29.42 KB 29.42 KB (0%) 30 KB
Service Worker - Legacy - NPM (CJS) 29.61 KB 29.61 KB (0%) 30 KB
Service Worker - Legacy - NPM (UMD) 29.44 KB 29.44 KB (0%) 30 KB
Service Worker - Modern - NPM (ESM) 24.59 KB 24.59 KB (0%) 25 KB
Service Worker - Modern - NPM (CJS) 24.85 KB 24.85 KB (0%) 25 KB
Service Worker - Modern - NPM (UMD) 24.64 KB 24.64 KB (0%) 25 KB
Core (v1.1) - NPM (ESM) 29.79 KB 29.79 KB (0%) 32 KB
Core (v1.1) - NPM (CJS) 29.92 KB 29.92 KB (0%) 32 KB
Core (v1.1) - NPM (UMD) 29.86 KB 29.86 KB (0%) 32 KB
Core (Content Script - v1.1) - NPM (ESM) 29.33 KB 29.33 KB (0%) 30 KB
Core (Content Script - v1.1) - NPM (CJS) 29.55 KB 29.55 KB (0%) 30 KB
Core (Content Script - v1.1) - NPM (UMD) 29.36 KB 29.36 KB (0%) 30 KB
Core - Legacy - CDN 47.42 KB 47.42 KB (0%) 48 KB
Core - Modern - CDN 24.09 KB 24.09 KB (0%) 24.5 KB
Load Snippet 724 B 718 B (-0.83% ▼) 1 KB
Core - Legacy - NPM (ESM) 47.22 KB 47.22 KB (0%) 47.5 KB
Core - Legacy - NPM (CJS) 47.46 KB 47.46 KB (0%) 48 KB
Core - Legacy - NPM (UMD) 47.3 KB 47.3 KB (0%) 47.5 KB
Core - Modern - NPM (ESM) 23.8 KB 23.8 KB (0%) 24.5 KB
Core - Modern - NPM (CJS) 24.05 KB 24.05 KB (0%) 24.5 KB
Core - Modern - NPM (UMD) 23.86 KB 23.86 KB (0%) 24.5 KB
Core (Bundled) - Legacy - NPM (ESM) 47.22 KB 47.22 KB (0%) 47.5 KB
Core (Bundled) - Legacy - NPM (CJS) 47.55 KB 47.55 KB (0%) 48 KB
Core (Bundled) - Legacy - NPM (UMD) 47.3 KB 47.3 KB (0%) 47.5 KB
Core (Bundled) - Modern - NPM (ESM) 38.57 KB 38.57 KB (0%) 39 KB
Core (Bundled) - Modern - NPM (CJS) 38.84 KB 38.84 KB (0%) 39 KB
Core (Bundled) - Modern - NPM (UMD) 38.58 KB 38.58 KB (0%) 39 KB
Core (Content Script) - Legacy - NPM (ESM) 46.71 KB 46.71 KB (0%) 47 KB
Core (Content Script) - Legacy - NPM (CJS) 47 KB 47 KB (0%) 47.5 KB
Core (Content Script) - Legacy - NPM (UMD) 46.78 KB 46.78 KB (0%) 47 KB
Core (Content Script) - Modern - NPM (ESM) 38.02 KB 38.02 KB (0%) 38.5 KB
Core (Content Script) - Modern - NPM (CJS) 38.28 KB 38.28 KB (0%) 38.5 KB
Core (Content Script) - Modern - NPM (UMD) 38.1 KB 38.1 KB (0%) 38.5 KB
All Integrations - Legacy - CDN 93.92 KB 93.92 KB (0%) 95.3 KB
All Integrations - Modern - CDN 88.8 KB 88.8 KB (0%) 91 KB

Copy link

sonarqubecloud bot commented Aug 5, 2024

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 389ad8f and 38d75ec.

Files selected for processing (14)
  • packages/analytics-js/public/index.html (1 hunks)
  • packages/loading-scripts/src/index.ts (1 hunks)
  • packages/sanity-suite/public/v3/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/index-npm.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/integrations/index-npm.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html (1 hunks)
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm.html (1 hunks)
Files skipped from review as they are similar to previous changes (11)
  • packages/analytics-js/public/index.html
  • packages/loading-scripts/src/index.ts
  • packages/sanity-suite/public/v3/index-cdn.html
  • packages/sanity-suite/public/v3/index-local.html
  • packages/sanity-suite/public/v3/index-npm-bundled.html
  • packages/sanity-suite/public/v3/index-npm.html
  • packages/sanity-suite/public/v3/integrations/index-cdn.html
  • packages/sanity-suite/public/v3/integrations/index-local.html
  • packages/sanity-suite/public/v3/integrations/index-npm.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html
  • packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html
Additional comments not posted (3)
packages/sanity-suite/public/v3/integrations/index-npm-bundled.html (1)

84-105: LGTM! But verify the function usage in the codebase.

The code changes are approved. The approach safely defines globalThis without modifying Object.prototype.

However, ensure that all references to globalThis are correctly handled across the codebase.

Verification successful

LGTM! But verify the function usage in the codebase.

The code changes are approved. The approach safely defines globalThis without modifying Object.prototype.

  • globalThis references are correctly handled across the codebase in various contexts, including setting properties, using global methods, and checking global objects.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis`. Expect: Correct usage across the codebase.
rg --type javascript -A 5 $'globalThis'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis` in .js and .jsx files. Expect: Correct usage across the codebase.
rg 'globalThis' --glob '**/*.js' --glob '**/*.jsx' -A 5

Length of output: 309359

packages/sanity-suite/public/v3/manualLoadCall/index-npm.html (1)

84-105: LGTM! But verify the function usage in the codebase.

The code changes are approved. The approach safely defines globalThis without modifying Object.prototype.

However, ensure that all references to globalThis are correctly handled across the codebase.

Verification successful

All references to globalThis are correctly handled across the codebase.

The search results show that globalThis is used appropriately in various contexts, such as checking for its existence, defining it, and utilizing it in different scripts and modules.

  • File: packages/sanity-suite/src/testBook/string.js

    • Usage: return globalThis.btoa(binString);
    • Context: This usage ensures that the btoa method is called from the global context, which is correct.
  • File: packages/sanity-suite/public/v3/index-npm.html

    • Usage: Several instances where globalThis is defined and checked.
    • Context: This ensures that globalThis is polyfilled if not available, which is a correct approach.
  • File: packages/sanity-suite/public/v3/manualLoadCall/index-npm.html

    • Usage: Similar to index-npm.html, ensuring globalThis is defined if not available.
  • File: packages/analytics-js/src/components/utilities/page.ts

    • Usage: Multiple usages like globalThis.navigator and globalThis.location.
    • Context: These usages are correct as they access global properties.
  • File: examples/chrome-extension/content-script-v3/foreground.js

    • Usage: Multiple instances of globalThis for accessing global properties and methods.
    • Context: Correctly used to ensure the code runs in the global context.
  • File: examples/chrome-extension/background-script-websockets/rudderAnalytics.js

    • Usage: const _global=(()=>{/*eslint no-undef:0*/if(typeof globalThis!=="undefined")return globalThis;
    • Context: This snippet ensures compatibility across different environments by checking for globalThis.
  • File: examples/chrome-extension/background-script/rudderAnalytics.js

    • Usage: Similar to rudderAnalytics.js in the websockets directory.
  • File: examples/chrome-extension/content-script-v1.1/foreground.js

    • Usage: Multiple instances of globalThis for accessing global properties and methods.
    • Context: Correctly used to ensure the code runs in the global context.

Category:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis`. Expect: Correct usage across the codebase.
rg --type javascript -A 5 $'globalThis'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis`. Expect: Correct usage across the codebase.
rg -A 5 $'globalThis'

Length of output: 406716

packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1)

84-105: LGTM! But verify the function usage in the codebase.

The code changes are approved. The approach safely defines globalThis without modifying Object.prototype.

However, ensure that all references to globalThis are correctly handled across the codebase.

Verification successful

Verification Successful: All references to globalThis are correctly handled across the codebase.

The search results indicate that globalThis is used consistently and appropriately in various files and contexts throughout the codebase.

  • Files and lines with globalThis usage:
    • jest/jest.setup-dom.js
    • packages/sanity-suite/src/testBook/string.js
    • examples/chrome-extension/background-script/rudderAnalytics.js
    • examples/chrome-extension/background-script-websockets/rudderAnalytics.js
    • examples/chrome-extension/content-script-v3/foreground.js
    • packages/analytics-js/src/testBook/string.js
    • packages/analytics-js/src/services/StoreManager/top-domain/index.ts
    • packages/analytics-js/src/services/StoreManager/storages/sessionStorage.ts
    • packages/analytics-js/src/services/StoreManager/storages/defaultOptions.ts
    • packages/analytics-js/src/services/ErrorHandler/ErrorHandler.ts
    • packages/analytics-js/src/components/utilities/page.ts
    • packages/analytics-js/src/components/utilities/globals.ts
    • packages/analytics-js/src/components/userSessionManager/UserSessionManager.ts
    • packages/analytics-js/src/components/preloadBuffer/index.ts
    • packages/analytics-js/src/components/eventRepository/EventRepository.ts
    • packages/analytics-js/src/components/eventManager/utilities.ts
    • packages/analytics-js/src/app/RudderAnalytics.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/storage.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/browser.ts
    • packages/analytics-js/src/components/capabilitiesManager/CapabilitiesManager.ts
    • packages/analytics-js/__tests__/nativeSdkLoader.js
    • packages/analytics-js/src/components/capabilitiesManager/detection/screen.ts
    • packages/analytics-js/src/components/capabilitiesManager/detection/dom.ts
    • packages/analytics-js/src/components/core/utilities.ts
    • packages/analytics-js/src/components/core/Analytics.ts
    • packages/loading-scripts/src/index.ts
    • packages/analytics-js/__tests__/components/utilities/page.test.ts
    • packages/analytics-js-cookies/src/component-cookie/index.ts
    • packages/analytics-js-cookies/__tests__/cookieUtilities.test.ts
    • packages/analytics-js-plugins/src/oneTrustConsentManager/index.ts
    • packages/analytics-js-plugins/src/utilities/retryQueue/Schedule.ts
    • packages/analytics-js-plugins/src/utilities/retryQueue/RetryQueue.ts
    • packages/analytics-js-plugins/src/errorReporting/utils.ts
    • packages/analytics-js-plugins/src/googleLinker/base64Decoder.ts
    • packages/analytics-js-plugins/src/deviceModeDestinations/utils.ts
    • packages/analytics-js-plugins/__tests__/errorReporting/utils.test.ts
    • packages/analytics-js-plugins/src/ketchConsentManager/index.ts
    • packages/analytics-js-plugins/src/bugsnag/utils.ts
    • packages/analytics-js-plugins/__tests__/bugsnag/utils.test.ts
    • packages/analytics-js-common/src/v1.1/utils/storage/v3DecryptionUtils.js
    • packages/analytics-js-common/src/services/ExternalSrcLoader/jsFileLoader.ts
    • packages/analytics-js-common/src/utilities/url.ts
    • packages/analytics-js-common/src/utilities/string.ts
    • packages/analytics-js-common/src/utilities/page.ts
    • assets/integrations/AdobeAnalytics/adobe-analytics-js.js
    • packages/analytics-js-common/src/utilities/crypto.ts
    • assets/integrations/AdobeAnalytics/adobe-analytics-js-heartbeat.js
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis`. Expect: Correct usage across the codebase.
rg --type javascript -A 5 $'globalThis'

Length of output: 73


Script:

#!/bin/bash
# Description: Verify all references to `globalThis` are correctly handled.

# Test: Search for references to `globalThis` in JavaScript and TypeScript files.
rg --glob '*.js' --glob '*.jsx' --glob '*.ts' --glob '*.tsx' -A 5 'globalThis'

Length of output: 362893

@saikumarrs saikumarrs merged commit 15acf40 into develop Aug 5, 2024
10 checks passed
@saikumarrs saikumarrs deleted the fix.loading-snippet-sdk-2239 branch August 5, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants