fix: replace polyfillio with fastly #1664
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe overall change involves updating the source URL for polyfill scripts across various files, switching from Changes
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 Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1664 +/- ##
========================================
Coverage 53.89% 53.89%
========================================
Files 461 461
Lines 15588 15588
Branches 3096 3107 +11
========================================
Hits 8401 8401
+ Misses 5871 5854 -17
- Partials 1316 1333 +17 ☔ View full report in Codecov by Sentry. |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (23)
- packages/analytics-js/public/index-legacy-only.html (1 hunks)
- packages/analytics-js/public/index.html (1 hunks)
- packages/analytics-js/rollup.config.mjs (1 hunks)
- packages/analytics-v1.1/rollup-configs/rollup.utilities.mjs (1 hunks)
- packages/loading-scripts/src/index.ts (2 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-cdn.html (1 hunks)
- packages/sanity-suite/public/v3/index-local.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.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.html (1 hunks)
Additional comments: 24
packages/loading-scripts/src/index.ts (2)
- 58-58: The globalThis polyfill implementation is a clever workaround for legacy Safari browsers. However, it's important to note that modifying
Object.prototypecan have unintended side effects in some cases. While this approach is generally safe, it's worth considering alternative polyfill strategies that avoid extending native prototypes if possible.- 86-86: The update to use
https://polyfill-fastly.io/v3/polyfill.min.jsis consistent with the PR's objective. However, it's crucial to verify that all features required by the application are supported by the Fastly polyfill service to ensure compatibility across all targeted browsers.packages/sanity-suite/public/v1.1/index-npm.html (1)
- 85-85: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/sanity-suite/public/v3/index-npm.html (1)
- 66-66: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/sanity-suite/public/v1.1/index-cdn.html (1)
- 109-109: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/sanity-suite/public/v1.1/index-local.html (1)
- 116-116: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/sanity-suite/public/v1.1/integrations/index-npm.html (1)
- 71-71: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/sanity-suite/public/v1.1/manualLoadCall/index-npm.html (1)
- 71-71: The update to the polyfill URL to
https://polyfill-fastly.io/v3/polyfill.min.jsis correctly implemented. Ensure that the specified features in the query parameters are fully supported by the Fastly service for all targeted browsers.packages/analytics-v1.1/rollup-configs/rollup.utilities.mjs (1)
- 24-24: The update to the
polyfillIoUrlvariable to usehttps://polyfill-fastly.io/v3/polyfill.min.jsaligns with the PR's objective. It's important to ensure that all necessary polyfill features are supported by the Fastly service for compatibility across all targeted browsers.packages/sanity-suite/public/v3/integrations/index-npm.html (1)
- 57-57: The update from
polyfill.iotopolyfill-fastly.iois correctly implemented according to the PR objectives. Ensure that the version and features specified in the URL are compatible and necessary for the project.packages/sanity-suite/public/v3/manualLoadCall/index-npm.html (1)
- 57-57: The transition to
polyfill-fastly.iois correctly applied here as well. It's important to verify that all specified features in the URL are indeed required for the project's functionality across supported browsers.packages/sanity-suite/public/v3/index-local.html (1)
- 66-66: The update to use
polyfill-fastly.iois correctly implemented. As with the previous files, ensure the specified features are necessary and that the version is the most appropriate for the project's needs.packages/sanity-suite/public/v3/index-cdn.html (1)
- 66-66: The switch to
polyfill-fastly.iois correctly done. It's good practice to periodically review the list of features requested from the polyfill service to ensure they are all still needed and to potentially optimize loading times.packages/sanity-suite/public/v1.1/integrations/index-cdn.html (1)
- 99-99: The update to
polyfill-fastly.iois correctly applied. It's advisable to ensure that the polyfill features specified are aligned with the project's browser support policy.packages/sanity-suite/public/v1.1/manualLoadCall/index-cdn.html (1)
- 99-99: Correct implementation of the switch to
polyfill-fastly.io. As always, verify that the features requested from the polyfill service are essential for the project.packages/sanity-suite/public/v1.1/integrations/index-local.html (1)
- 101-101: The transition to
polyfill-fastly.iois correctly done here as well. Ensure that the polyfill features included in the URL are still relevant and necessary for the project.packages/sanity-suite/public/v1.1/manualLoadCall/index-local.html (1)
- 101-101: The update of the polyfill service URL from
polyfill.iotopolyfill-fastly.iois correctly implemented. This change aligns with the PR's objective to transition to Fastly's polyfill service for potentially improved performance or reliability. Ensure that this new URL is tested across the specified browsers (Chrome, Firefox, IE11) as part of the cross-browser testing checklist to confirm compatibility and functionality.packages/sanity-suite/public/v3/integrations/index-cdn.html (1)
- 66-66: The update from
polyfill.iotopolyfill-fastly.ioaligns with the PR's objective. Ensure that the specified version (3.111.0) and features (Symbol,Promise) are supported and behave as expected on Fastly's service.packages/sanity-suite/public/v3/manualLoadCall/index-cdn.html (1)
- 66-66: The change to
polyfill-fastly.iois consistent with the PR's goal. Verify that theversion=3.111.0and the featuresSymbolandPromiseare fully compatible and performant with Fastly's polyfill service.packages/sanity-suite/public/v3/integrations/index-local.html (1)
- 66-66: The switch to
polyfill-fastly.iois correctly implemented here. As with the other files, ensure compatibility and performance of the specifiedversion=3.111.0and features (Symbol,Promise) with Fastly's service.packages/sanity-suite/public/v3/manualLoadCall/index-local.html (1)
- 66-66: The update to
polyfill-fastly.iois correctly applied. Confirm thatversion=3.111.0and the requested features (Symbol,Promise) work as expected with Fastly's polyfill service.packages/analytics-js/public/index-legacy-only.html (1)
- 65-65: The transition to
polyfill-fastly.iois properly executed in this file. Ensure that theversion=3.111.0and features (Symbol,Promise) specified are compatible and perform as expected with Fastly's polyfill service.packages/analytics-js/public/index.html (1)
- 65-65: The update of the polyfill service URL from
polyfill.iotopolyfill-fastly.iois noted. Ensure that comprehensive cross-browser testing, as mentioned in the PR description, has been conducted to verify that this change does not adversely affect the application's functionality across different environments.packages/analytics-js/rollup.config.mjs (1)
- 47-47: The update of the
polyfillIoUrlvariable frompolyfill.iotopolyfill-fastly.ioin the Rollup configuration is noted. Ensure that the bundled output has been verified for correctness and that comprehensive cross-browser testing, as mentioned in the PR description, has been conducted to ensure that this change does not adversely affect the application's functionality across different environments.
size-limit report 📦
|
MoumitaM
approved these changes
Mar 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
We're now using fastly polyfill service instead of polyfill.io.
Linear task (optional)
https://linear.app/rudderstack/issue/SDK-1405/use-fastly-instead-of-polyfillio
Cross Browser Tests
Please confirm you have tested for the following browsers:
Sanity Suite
Security
Summary by CodeRabbit
polyfill-fastly.io) for improved compatibility and performance. This affects the loading of essential web features in legacy and modern browsers alike.