feat: Remove img, jsonp, and xhrGet methods #576
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
Codecov Report
@@ Coverage Diff @@
## main #576 +/- ##
==========================================
+ Coverage 66.74% 68.33% +1.58%
==========================================
Files 131 129 -2
Lines 6021 5960 -61
Branches 1121 1120 -1
==========================================
+ Hits 4019 4073 +54
+ Misses 1626 1532 -94
+ Partials 376 355 -21
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
patrickhousley
force-pushed
the
remove-img-network
branch
from
8bf9cd4
to
9c4eda4
Compare
patrickhousley
force-pushed
the
remove-img-network
branch
from
9c4eda4
to
c0352a2
Compare
Asset Size ReportMerging this pull request will result in the following CDN asset size changes:
Merging this pull request will result in the following NPM package consumer size changes:
Other Standard CDN AssetsReleased Assets
Built Assets
Other Polyfill CDN AssetsReleased Assets
Built Assets
|
patrickhousley
commented
Jun 9, 2023
patrickhousley
force-pushed
the
remove-img-network
branch
4 times, most recently
from
175825e
to
32bcf4b
Compare
patrickhousley
force-pushed
the
remove-img-network
branch
from
32bcf4b
to
7f93150
Compare
cwli24
reviewed
Jun 16, 2023
patrickhousley
force-pushed
the
remove-img-network
branch
6 times, most recently
from
1ab8870
to
b2b8fbb
Compare
patrickhousley
force-pushed
the
remove-img-network
branch
from
b2b8fbb
to
8169bff
Compare
bjfield
changed the title
metal-messiah
approved these changes
Jun 21, 2023
There was a problem hiding this comment.
I think this covers all the topics mentioned and I have not been able to find any faults from the migration of the previous implementation. This still likely represents a change that affects the entire agent, so I think we should consider deploying this as a standalone shipment, but i think it looks G2G. approved!
|
To clarify since it wasn't explicitly stated elsewhere, this will remove the ability for IE11 (those still on it) to send out data on page unload events, yes? |
Not entirely. For IE11, the img method was used before and now XHR will be used. Both methods are a best effort and we cannot guarantee that either will actually result in the data making it out of the browser before the browser kills the JS process. |
bjfield
approved these changes
Jun 21, 2023
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.
Unused agent network methods have been removed. All data transmitted by the agent will now default to using XMLHttpRequest POST calls except for data transmitted during a visibility change or page unload. Visibility changes and page unloads will use sendBeacon with a fallback to fetch with keepalive. For browsers that do not support sendBeacon, the final data transmission will use XMLHttpRequest POST.
Overview
Removing the unused
sendDatamethodsjsonp,img', andxhrGet. All data will now be transmitted usingXMLHttpRequestviaPOST. For browsers that supportsendBeacon, the final harvest will fallback tofetchwithkeepalive: true`.All tests for
harvest.jsandharvest-scheduler.jsmodules have been converted to Jest and WDIO. In doing so, I have created many auto-mock files in__mock__directories. Using this style of mocking modules will let us manage the mock in a central location for each module and eventually enabled auto mocking globally for our jest unit tests.Minor cleanup work was done with the
util/global-scope.jsandbrowser-version/*.jsmodules. Since these modules exported constants that were determined at runtime, I moved them into thecommon/constantsfolder into a single module. This will reduce import/require statements in the final bundle.The pull request checks workflow has been updated to add the size check comment to PRs. The babel and ts configs have been updated so the test files and mocks do not get included in our
/dist/directory.Related Issue(s)
Functional test conversions:
Browser test conversions:
Testing
Unit and integration tests have been updated.