fix: npm sanity suites #1810
fix: npm sanity suites #1810
Conversation
WalkthroughWalkthroughThe recent updates enhance GitHub Actions workflows by incorporating additional Slack notifications for new releases, broadening their audience through new user group mentions. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHubActions
participant Slack
User->>GitHubActions: Trigger Release Workflow
GitHubActions->>Slack: Notify Release with Ticket ID
Slack-->>User: Notify relevant user groups
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1810 +/- ##
===========================================
+ Coverage 56.54% 56.61% +0.07%
===========================================
Files 473 473
Lines 16114 16093 -21
Branches 3212 3224 +12
===========================================
Hits 9111 9111
+ Misses 5788 5734 -54
- Partials 1215 1248 +33 ☔ View full report in Codecov by Sentry. |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
packages/analytics-js-common/src/v1.1/utils/index.ts (1)
1-1: Issues found due to the removal ofintegration_cname.The removal of
integration_cnamewill break dependencies in the following files:
packages/analytics-v1.1/src/core/analytics.jspackages/analytics-v1.1/src/utils/utils.jsPlease ensure to address these dependencies before proceeding.
Analysis chain
Verify the removal of
integration_cname.Ensure that the removal of
integration_cnamedoes not break any dependencies in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the removal of `integration_cname` breaks any dependencies. # Test: Search for the usage of `integration_cname`. Expect: No occurrences. rg --type python -A 5 $'integration_cname'Length of output: 72
Script:
#!/bin/bash # Description: Verify if the removal of `integration_cname` breaks any dependencies. # Test: Search for the usage of `integration_cname`. Expect: No occurrences. rg 'integration_cname' -A 5Length of output: 5747
packages/analytics-v1.1/src/core/analytics.js (1)
18-19: Incorrect import paths forconfigToIntNamesandcommonNamesThe files
config_to_integration_names.jsandintegration_cname.jsdo not exist at the specified paths@rudderstack/analytics-js-common/constants/integrations/config_to_integration_namesand@rudderstack/analytics-js-common/constants/integrations/integration_cname.
- Please verify and correct the import paths for these modules.
Analysis chain
LGTM! But verify the new import paths.
The import paths for
configToIntNamesandcommonNameshave been updated correctly.However, ensure that the new paths
@rudderstack/analytics-js-common/constants/integrations/config_to_integration_namesand@rudderstack/analytics-js-common/constants/integrations/integration_cnameare correct and valid.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new import paths are correct and valid. # Test: Search for the imported modules in the new paths. Expect: Only valid paths. rg --type js $'@rudderstack/analytics-js-common/constants/integrations/config_to_integration_names' rg --type js $'@rudderstack/analytics-js-common/constants/integrations/integration_cname'Length of output: 858
Script:
#!/bin/bash # Description: Verify the existence of the files at the new import paths. # Test: Check if the files exist at the specified paths. Expect: Files should exist. if [ -f "packages/analytics-js-common/constants/integrations/config_to_integration_names.js" ]; then echo "config_to_integration_names.js exists"; else echo "config_to_integration_names.js does not exist"; fi if [ -f "packages/analytics-js-common/constants/integrations/integration_cname.js" ]; then echo "integration_cname.js exists"; else echo "integration_cname.js does not exist"; fiLength of output: 363
Tools
GitHub Check: codecov/patch
[warning] 18-19: packages/analytics-v1.1/src/core/analytics.js#L18-L19
Added lines #L18 - L19 were not covered by tests
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.jsonpackages/sanity-suite/package.jsonis excluded by!**/*.json
Files selected for processing (31)
- .github/workflows/deploy-npm.yml (6 hunks)
- .github/workflows/deploy-sanity-suite.yml (2 hunks)
- .github/workflows/deploy.yml (2 hunks)
- .github/workflows/draft-new-release.yml (4 hunks)
- .github/workflows/publish-new-release.yml (3 hunks)
- CODEOWNERS (1 hunks)
- packages/analytics-js-common/src/constants/index.ts (1 hunks)
- packages/analytics-js-common/src/constants/integrations/destinationNames.ts (1 hunks)
- packages/analytics-js-common/src/constants/integrations/integration_cname.js (1 hunks)
- packages/analytics-js-common/src/v1.1/utils/index.ts (1 hunks)
- packages/analytics-js-integrations/scripts/integrationBuildScript.js (1 hunks)
- packages/analytics-js-plugins/src/deviceModeDestinations/index.ts (1 hunks)
- packages/analytics-v1.1/src/core/analytics.js (1 hunks)
- packages/analytics-v1.1/src/features/core/metrics/errorReporting/providers/Bugsnag.js (1 hunks)
- packages/analytics-v1.1/src/utils/utils.js (1 hunks)
- packages/sanity-suite/.env.example (1 hunks)
- packages/sanity-suite/public/v1.1/index-cdn.html (1 hunks)
- packages/sanity-suite/public/v1.1/index-local.html (1 hunks)
- packages/sanity-suite/public/v1.1/index-npm.html (1 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-cdn.html (1 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-local.html (1 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-npm.html (1 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html (1 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-local.html (1 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html (1 hunks)
- packages/sanity-suite/public/v3/index-local.html (1 hunks)
- packages/sanity-suite/public/v3/integrations/index-local.html (1 hunks)
- packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1 hunks)
- packages/sanity-suite/rollup.config.mjs (6 hunks)
- packages/sanity-suite/src/index-npm.ts (1 hunks)
- sonar-project.properties (1 hunks)
Files skipped from review due to trivial changes (17)
- .github/workflows/deploy-npm.yml
- CODEOWNERS
- packages/analytics-js-common/src/constants/index.ts
- packages/analytics-js-integrations/scripts/integrationBuildScript.js
- packages/analytics-js-plugins/src/deviceModeDestinations/index.ts
- packages/analytics-v1.1/src/features/core/metrics/errorReporting/providers/Bugsnag.js
- packages/analytics-v1.1/src/utils/utils.js
- packages/sanity-suite/public/v1.1/index-cdn.html
- packages/sanity-suite/public/v1.1/index-local.html
- packages/sanity-suite/public/v1.1/index-npm.html
- packages/sanity-suite/public/v1.1/integrations/index-cdn.html
- packages/sanity-suite/public/v1.1/integrations/index-local.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/manualLoadCall/index-local.html
- packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html
- packages/sanity-suite/src/index-npm.ts
Additional context used
GitHub Check: codecov/patch
packages/analytics-js-common/src/constants/integrations/integration_cname.js
[warning] 1-79: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L1-L79
Added lines #L1 - L79 were not covered by tests
[warning] 83-83: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L83
Added line #L83 was not covered by tests
[warning] 166-166: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L166
Added line #L166 was not covered by testspackages/analytics-js-common/src/constants/integrations/destinationNames.ts
[warning] 45-45: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L45
Added line #L45 was not covered by tests
[warning] 54-55: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L54-L55
Added lines #L54 - L55 were not covered by tests
[warning] 72-72: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L72
Added line #L72 was not covered by tests
[warning] 85-85: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L85
Added line #L85 was not covered by tests
[warning] 166-166: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L166
Added line #L166 was not covered by tests
[warning] 183-183: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L183
Added line #L183 was not covered by tests
[warning] 220-220: packages/analytics-js-common/src/constants/integrations/destinationNames.ts#L220
Added line #L220 was not covered by testspackages/analytics-v1.1/src/core/analytics.js
[warning] 18-19: packages/analytics-v1.1/src/core/analytics.js#L18-L19
Added lines #L18 - L19 were not covered by tests
Biome
packages/sanity-suite/rollup.config.mjs
[error] 57-57: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (37)
packages/sanity-suite/.env.example (6)
3-3: Verify the necessity of an empty value forCONFIG_SERVER_HOST.An empty value for
CONFIG_SERVER_HOSTcan cause issues if the application expects a valid URL. Ensure this is intentional or provide a default value.
4-4: Verify the necessity of an empty value forSDK_CDN_VERSION_PATH_PREFIX.An empty value for
SDK_CDN_VERSION_PATH_PREFIXcan cause issues if the application expects a valid path prefix. Ensure this is intentional or provide a default value.
5-5: Verify the necessity of an empty value forDEST_SDK_BASE_URL.An empty value for
DEST_SDK_BASE_URLcan cause issues if the application expects a valid base URL. Ensure this is intentional or provide a default value.
8-8: Verify the necessity of commenting outPLUGINS_SDK_BASE_URL.Commenting out
PLUGINS_SDK_BASE_URLmight indicate it is no longer needed or temporarily disabled. Ensure this change is intentional.
11-11: Verify the necessity of commenting outIS_DEV_TESTBOOK.Commenting out
IS_DEV_TESTBOOKmight indicate it is no longer needed or temporarily disabled. Ensure this change is intentional.
14-14: Verify the necessity of commenting outIS_DMT.Commenting out
IS_DMTmight indicate it is temporarily disabled. Ensure this change is intentional.packages/analytics-js-common/src/v1.1/utils/index.ts (2)
1-1: Verify the removal ofconfig_to_integration_names.Ensure that the removal of
config_to_integration_namesdoes not break any dependencies in the codebase.Verification successful
Let's correct the file type and search for the usage of
config_to_integration_namesin TypeScript files.
The removal of
config_to_integration_namesdoes not break any dependencies.The search for
config_to_integration_namesin the TypeScript files returned no results, indicating that this export is not used elsewhere in the codebase.
- No occurrences of
config_to_integration_nameswere found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the removal of `config_to_integration_names` breaks any dependencies. # Test: Search for the usage of `config_to_integration_names`. Expect: No occurrences. rg --type python -A 5 $'config_to_integration_names'Length of output: 82
Script:
#!/bin/bash # Description: Verify if the removal of `config_to_integration_names` breaks any dependencies. # Test: Search for the usage of `config_to_integration_names` in TypeScript files. Expect: No occurrences. rg --type ts -A 5 $'config_to_integration_names'Length of output: 47
1-1: Verify the removal ofclient_server_name.Ensure that the removal of
client_server_namedoes not break any dependencies in the codebase.Verification successful
The removal of
client_server_namedoes not break any dependencies.The search for occurrences of
client_server_namewithin the TypeScript files did not produce any results, indicating that it is not used elsewhere in the codebase.
- No occurrences of
client_server_namefound in the TypeScript files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the removal of `client_server_name` breaks any dependencies. # Test: Search for the usage of `client_server_name`. Expect: No occurrences. rg --type python -A 5 $'client_server_name'Length of output: 73
Script:
#!/bin/bash # Description: Verify if the removal of `client_server_name` breaks any dependencies. # Test: Search for the usage of `client_server_name`. Expect: No occurrences. rg --type ts -A 5 'client_server_name'Length of output: 38
sonar-project.properties (2)
31-31: Verify the necessity of excludingpackages/analytics-js-common/src/constants/integrations/**/*from coverage analysis.Ensure that excluding this path from coverage analysis is intentional and does not omit critical code from testing.
37-37: Verify the necessity of excludingpackages/analytics-js-common/src/constants/integrations/**/*from copy-paste detection.Ensure that excluding this path from copy-paste detection is intentional and does not omit critical code from analysis.
.github/workflows/draft-new-release.yml (3)
5-8: LGTM! New required input added correctly.The new required input
release_ticket_idis correctly defined for theworkflow_dispatchtrigger.
61-61: LGTM! Branch naming convention updated.The branch naming convention now includes the
release_ticket_id, which improves traceability.
106-110: LGTM! PR body updated to include linear ticket reference.The PR body now includes a reference to the linear ticket, improving documentation and traceability.
.github/workflows/publish-new-release.yml (2)
33-36: LGTM! Suffix removal from version string implemented correctly.The
awkcommand correctly removes the-SDK-<ticket_number>suffix from the version string, ensuring versioning consistency.
119-119: LGTM! Slack notification payload updated.The Slack notification payload now includes an additional subteam mention, broadening the reach of release notifications.
Also applies to: 135-135
.github/workflows/deploy-sanity-suite.yml (1)
154-154: LGTM! Slack notification payload updated.The Slack notification payload now includes an additional subteam mention, broadening the reach of deployment notifications.
Also applies to: 170-170
packages/analytics-js-common/src/constants/integrations/integration_cname.js (2)
1-80: LGTM!The import statements are well-organized and correctly reference the constants from various integrations.
Tools
GitHub Check: codecov/patch
[warning] 1-79: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L1-L79
Added lines #L1 - L79 were not covered by tests
81-166: LGTM!The
commonNamesmapping is correctly defined and the export statement is appropriate.Tools
GitHub Check: codecov/patch
[warning] 83-83: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L83
Added line #L83 was not covered by tests
[warning] 166-166: packages/analytics-js-common/src/constants/integrations/integration_cname.js#L166
Added line #L166 was not covered by testspackages/sanity-suite/public/v3/index-local.html (2)
Line range hint
1-119:
LGTM!The HTML structure is well-formed and correctly defines the RudderStack JS SDK Sanity Suite.
120-122: LGTM!The changes to the SDK base URLs are appropriate and improve the consistency and predictability of the configuration.
packages/sanity-suite/rollup.config.mjs (3)
19-21: LGTM!The variable declarations are correct and the renaming to uppercase constants improves clarity and readability.
63-76: LGTM!The changes to the URL construction logic are appropriate and improve robustness.
78-89: LGTM!The changes to the URL construction logic are appropriate and improve robustness.
packages/sanity-suite/public/v3/integrations/index-local.html (2)
109-109: LGTM! Simplified URL configuration.The
destSDKBaseURLhas been simplified to a static value, which reduces complexity and potential errors.
111-111: LGTM! Simplified URL configuration.The
pluginsSDKBaseURLhas been simplified to a static value, which reduces complexity and potential errors.packages/sanity-suite/public/v3/manualLoadCall/index-local.html (2)
106-106: LGTM! Simplified URL configuration.The
destSDKBaseURLhas been simplified to a static value, which reduces complexity and potential errors.
108-108: LGTM! Simplified URL configuration.The
pluginsSDKBaseURLhas been simplified to a static value, which reduces complexity and potential errors.packages/analytics-js-common/src/constants/integrations/destinationNames.ts (8)
4-4: LGTM! Simplified import path.The import path for
AdobeAnalyticshas been simplified, improving clarity and maintainability.
8-8: LGTM! Simplified import path.The import path for
Amplitudehas been simplified, improving clarity and maintainability.
12-12: LGTM! Simplified import path.The import path for
Appcueshas been simplified, improving clarity and maintainability.
16-16: LGTM! Simplified import path.The import path for
BingAdshas been simplified, improving clarity and maintainability.
20-20: LGTM! Simplified import path.The import path for
Brazehas been simplified, improving clarity and maintainability.
24-24: LGTM! Simplified import path.The import path for
Bugsnaghas been simplified, improving clarity and maintainability.
49-49: LGTM! Simplified import path.The import path for
FacebookPixelhas been simplified, improving clarity and maintainability.
53-53: LGTM! Simplified import path.The import path for
Fullstoryhas been simplified, improving clarity and maintainability..github/workflows/deploy.yml (2)
186-186: LGTM! But verify the new Slack subteam mention.The new subteam mention
<!subteam^S07CCQ05F2T>is correctly added to the "text" field.However, ensure that the subteam ID
S07CCQ05F2Tis correct and valid.
202-202: LGTM! But verify the new Slack subteam mention.The new subteam mention
<!subteam^S07CCQ05F2T>is correctly added to the "blocks" field.However, ensure that the subteam ID
S07CCQ05F2Tis correct and valid.
saikumarrs
requested review from
ItsSudip,
shrouti1507,
sandeepdsvs,
koladilip and
a team
as code owners
|
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (1)
packages/sanity-suite/public/v3/index-npm-bundled.html (1)
1-24: Improve readability of OneTrust script.Format the OneTrust script for better readability.
- <script> - window.OneTrust = { - GetDomainData: function () { - return { - Groups: [ - { CustomGroupId: 'C0001', GroupName: 'Functional Cookies' }, - { CustomGroupId: 'C0002', GroupName: 'Performance Cookies' }, - { CustomGroupId: 'C0003', GroupName: 'Analytical Cookies' }, - { CustomGroupId: 'C0004', GroupName: 'Targeting Cookies' }, - { CustomGroupId: 'C0005', GroupName: 'Social Media Cookies' }, - { CustomGroupId: 'C0006', GroupName: 'Advertisement Cookies' }, - ], - }; - }, - }; - window.OnetrustActiveGroups = ',C0001,C0003,'; - </script> + <script> + window.OneTrust = { + GetDomainData: function () { + return { + Groups: [ + { CustomGroupId: 'C0001', GroupName: 'Functional Cookies' }, + { CustomGroupId: 'C0002', GroupName: 'Performance Cookies' }, + { CustomGroupId: 'C0003', GroupName: 'Analytical Cookies' }, + { CustomGroupId: 'C0004', GroupName: 'Targeting Cookies' }, + { CustomGroupId: 'C0005', GroupName: 'Social Media Cookies' }, + { CustomGroupId: 'C0006', GroupName: 'Advertisement Cookies' }, + ], + }; + }, + }; + window.OnetrustActiveGroups = ',C0001,C0003,'; + </script>
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
packages/sanity-suite/package.jsonis excluded by!**/*.json
Files selected for processing (19)
- packages/sanity-suite/public/v1.1/index-cdn.html (2 hunks)
- packages/sanity-suite/public/v1.1/index-local.html (2 hunks)
- packages/sanity-suite/public/v1.1/index-npm.html (2 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-cdn.html (2 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-local.html (2 hunks)
- packages/sanity-suite/public/v1.1/integrations/index-npm.html (2 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html (2 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-local.html (2 hunks)
- packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html (2 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-npm-bundled.html (1 hunks)
- packages/sanity-suite/public/v3/integrations/index-npm.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)
- packages/sanity-suite/rollup.config.mjs (7 hunks)
- packages/sanity-suite/src/index-npm-bundled.ts (1 hunks)
- packages/sanity-suite/src/index-npm-v1.1.ts (1 hunks)
- packages/sanity-suite/src/index-npm.ts (1 hunks)
Files skipped from review due to trivial changes (4)
- packages/sanity-suite/public/v3/index-npm.html
- packages/sanity-suite/public/v3/integrations/index-npm.html
- packages/sanity-suite/public/v3/manualLoadCall/index-npm.html
- packages/sanity-suite/src/index-npm-v1.1.ts
Files skipped from review as they are similar to previous changes (8)
- packages/sanity-suite/public/v1.1/index-cdn.html
- packages/sanity-suite/public/v1.1/index-local.html
- packages/sanity-suite/public/v1.1/index-npm.html
- packages/sanity-suite/public/v1.1/integrations/index-cdn.html
- packages/sanity-suite/public/v1.1/integrations/index-local.html
- packages/sanity-suite/public/v1.1/integrations/index-npm.html
- packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html
- packages/sanity-suite/src/index-npm.ts
Additional context used
Biome
packages/sanity-suite/rollup.config.mjs
[error] 61-61: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Additional comments not posted (31)
packages/sanity-suite/src/index-npm-bundled.ts (1)
55-55: LGTM!The invocation of
sanitySuiteApplooks good.packages/sanity-suite/public/v3/index-npm-bundled.html (2)
27-57: LGTM!The RudderAnalytics snippet looks good.
122-129: LGTM!The body section looks good.
packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html (3)
4-4: LGTM!The new link tag for the favicon looks good.
72-72: LGTM!The updated script tag for polyfills looks good.
1-1: LGTM!The body section looks good.
packages/sanity-suite/public/v1.1/manualLoadCall/index-local.html (2)
4-4: Addition of favicon link looks good.The link to the favicon enhances the visual branding of the web page.
102-102: Modification of polyfill script source URL looks good.The updated URL includes additional features, enhancing compatibility across different environments.
packages/sanity-suite/public/v3/integrations/index-npm-bundled.html (7)
4-4: Addition of favicon link looks good.The link to the favicon enhances the visual branding of the web page.
9-25: Addition of OneTrust script looks good.The script is correctly implemented to manage cookie consent.
27-58: Addition of RudderStack snippet looks good.The snippet is correctly implemented to load the RudderStack SDK.
81-81: Addition of polyfill script source URL looks good.The URL includes necessary features, enhancing compatibility across different environments.
83-94: Addition of globalThis polyfill looks good.The polyfill ensures compatibility across different environments.
95-95: Addition of clipboard script looks good.The script is correctly implemented to enable clipboard functionality.
108-118: Addition of jQuery script looks good.The script is correctly implemented to enable jQuery functionality.
packages/sanity-suite/public/v3/manualLoadCall/index-npm-bundled.html (7)
4-4: Addition of favicon link looks good.The link to the favicon enhances the visual branding of the web page.
9-25: Addition of OneTrust script looks good.The script is correctly implemented to manage cookie consent.
27-58: Addition of RudderStack snippet looks good.The snippet is correctly implemented to load the RudderStack SDK.
81-81: Addition of polyfill script source URL looks good.The URL includes necessary features, enhancing compatibility across different environments.
83-94: Addition of globalThis polyfill looks good.The polyfill ensures compatibility across different environments.
95-95: Addition of clipboard script looks good.The script is correctly implemented to enable clipboard functionality.
108-118: Addition of jQuery script looks good.The script is correctly implemented to enable jQuery functionality.
packages/sanity-suite/rollup.config.mjs (9)
19-21: Constants definition improves readability.Defining
SERVER_PORT,BASE_CDN_URL, andDEFAULT_SDK_VERSIONas constants enhances code clarity and maintainability.
24-24: Variable renaming enhances clarity.Renaming
isLegacytoisLegacySdkmakes the code more readable by clearly indicating its purpose.
48-49: Consistent changes.The changes are consistent with the rest of the code.
58-59: Consistent use of renamed variable.The
getJSSourcefunction now uses theisLegacySdkvariable, maintaining consistency and improving readability.
67-82: Enhanced robustness ingetDestinationSDKBaseURL.The function now returns a default value when
DEST_SDK_BASE_URLis not set, enhancing robustness.
84-97: Improved clarity ingetPluginsBaseURL.The function now effectively uses the new constants, improving code clarity and maintainability.
106-109: Consistent changes.The changes are consistent with the rest of the code.
128-144: Consistent changes.The changes are consistent with the rest of the code.
Line range hint
171-216:
Consistent use of new constants in build configuration.The build configuration object now uses the
getDestinationSDKBaseURLandgetPluginsBaseURLfunctions, promoting consistency and reducing potential errors.
PR Description
Fixed the sanity suite package to correctly build it for the NPM variant.
I've also made several improvements:
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-2075/fix-issues-in-sanity-suites-of-the-npm-package-js-sdk
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
.env.exampleto reflect changes in SDK and plugin references.Style
Chores