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

fix: Refactor usage of Array.from to address MooTools conflict #544

Merged
merged 7 commits into from
May 26, 2023

Conversation

bjfield
Copy link
Contributor

@bjfield bjfield commented May 25, 2023

To work around a conflict with some versions of the MooTools library that mutate the native Array.from method, the agent will now use alternative methods.

Overview

In the 1.232.1 release, we removed lodash.slice, which was being used to convert iterables to arrays—using Array.from instead. It was discovered that some older versions of the MooTools library mutate the Array.from method in a way that caused pages to break when both libraries were used in combination. This PR:

  • Replaces instances of Array.from with spread operators (which will be transpiled by babel as needed).
  • Removes the core-js/stable/array/from polyfill (which was included only with polyfill builds of the agent).
    • Babel transpiles spreads to Array.from, requiring the polyfill.

Related Issue(s)

NR-119295

Testing

  • Standard tests should pass.
  • Various builds of MooTools should be tested manually.

@github-actions
Copy link

github-actions bot commented May 25, 2023

Asset Size Report

Merging this pull request will result in the following CDN asset size changes:

Asset Name Previous Size New Size Diff
nr-loader-spa.min 52.09 kB / 17.78 kB (gzip) 52.06 kB / 17.77 kB (gzip) -0.07% / -0.04% (gzip)
nr-loader-full.min 46.17 kB / 15.99 kB (gzip) 46.14 kB / 15.98 kB (gzip) -0.06% / -0.05% (gzip)
nr-loader-rum.min 27.36 kB / 10.08 kB (gzip) 27.35 kB / 10.07 kB (gzip) -0.05% / -0.05% (gzip)
nr-loader-spa-polyfills.min 118.49 kB / 39.15 kB (gzip) 125.15 kB / 40.09 kB (gzip) +5.62% / +2.42% (gzip)
nr-loader-full-polyfills.min 111.75 kB / 37.26 kB (gzip) 117.65 kB / 38.16 kB (gzip) +5.28% / +2.43% (gzip)
nr-loader-rum-polyfills.min 90.84 kB / 30.91 kB (gzip) 95.93 kB / 31.77 kB (gzip) +5.6% / +2.78% (gzip)
nr-loader-worker.min 42.23 kB / 14.65 kB (gzip) 42.2 kB / 14.65 kB (gzip) -0.07% / -0.05% (gzip)

Merging this pull request will result in the following NPM package consumer size changes:

Asset Name Previous Size New Size Diff
Browser Agent 52.16 kB / 17.65 kB (gzip) 52.13 kB / 17.64 kB (gzip) -0.07% / -0.03% (gzip)
Custom Lite Agent 27.45 kB / 9.99 kB (gzip) 27.44 kB / 9.99 kB (gzip) -0.05% / -0.04% (gzip)
Custom Pro Agent 46.16 kB / 15.8 kB (gzip) 46.13 kB / 15.79 kB (gzip) -0.06% / -0.05% (gzip)
Custom SPA Agent 52.04 kB / 17.63 kB (gzip) 52.01 kB / 17.63 kB (gzip) -0.07% / -0.05% (gzip)
Worker Agent 302.15 kB / 93.77 kB (gzip) 302.12 kB / 93.76 kB (gzip) -0.01% / -0% (gzip)
Other Standard CDN Assets

Released Assets

Asset Name Asset Size
recorder.b9a5173b.min.js 166.07 kB / 52.31 kB (gzip)
spa-aggregate.ec1a70de.min.js 18.62 kB / 6.63 kB (gzip)
286.a2dfc50f.min.js 13.98 kB / 5.2 kB (gzip)
page_view_timing-aggregate.ddd91465.min.js 12.7 kB / 4.64 kB (gzip)
page_view_event-aggregate.edc9f963.min.js 11.25 kB / 4.2 kB (gzip)
875.b62b7d24.min.js 10.1 kB / 4.07 kB (gzip)
session_trace-aggregate.d4b3966c.min.js 8.11 kB / 3.07 kB (gzip)
compressor.92c94011.min.js 7.09 kB / 3.57 kB (gzip)
jserrors-aggregate.cb5045c7.min.js 7.07 kB / 2.75 kB (gzip)
metrics-aggregate.80e5be98.min.js 6.22 kB / 2.06 kB (gzip)
ajax-aggregate.666f66ea.min.js 4.81 kB / 2.22 kB (gzip)
session_replay-aggregate.584320be.min.js 4.53 kB / 1.77 kB (gzip)
async-api.288afefe.min.js 3 kB / 1.49 kB (gzip)
page_action-aggregate.aef67c05.min.js 2.42 kB / 1.05 kB (gzip)
session-manager.677b9b5a.min.js 2.23 kB / 1.03 kB (gzip)
lazy-feature-loader.25ca4880.min.js 1.11 kB / 495 B (gzip)

Built Assets

Asset Name Asset Size
recorder.b9a5173b.min.js 166.07 kB / 52.31 kB (gzip)
spa-aggregate.ec1a70de.min.js 18.62 kB / 6.63 kB (gzip)
286.bd1fce31.min.js 13.96 kB / 5.19 kB (gzip)
page_view_timing-aggregate.ddd91465.min.js 12.7 kB / 4.64 kB (gzip)
page_view_event-aggregate.edc9f963.min.js 11.25 kB / 4.2 kB (gzip)
875.b62b7d24.min.js 10.1 kB / 4.07 kB (gzip)
session_trace-aggregate.d4b3966c.min.js 8.11 kB / 3.07 kB (gzip)
compressor.92c94011.min.js 7.09 kB / 3.57 kB (gzip)
jserrors-aggregate.cb5045c7.min.js 7.07 kB / 2.75 kB (gzip)
metrics-aggregate.80e5be98.min.js 6.22 kB / 2.06 kB (gzip)
ajax-aggregate.666f66ea.min.js 4.81 kB / 2.22 kB (gzip)
session_replay-aggregate.584320be.min.js 4.53 kB / 1.77 kB (gzip)
async-api.288afefe.min.js 3 kB / 1.49 kB (gzip)
page_action-aggregate.aef67c05.min.js 2.42 kB / 1.05 kB (gzip)
session-manager.677b9b5a.min.js 2.23 kB / 1.03 kB (gzip)
lazy-feature-loader.25ca4880.min.js 1.11 kB / 495 B (gzip)
Other Polyfill CDN Assets

Released Assets

Asset Name Asset Size
recorder.b9a5173b-es5.min.js 166.76 kB / 52.35 kB (gzip)
nr-polyfills.min.js 52.15 kB / 17.96 kB (gzip)
compressor.39f21c15-es5.min.js 30.02 kB / 11.29 kB (gzip)
spa-aggregate.a2d4bd8d-es5.min.js 19.54 kB / 6.79 kB (gzip)
286.22d28761-es5.min.js 15.76 kB / 5.75 kB (gzip)
page_view_timing-aggregate.0b0f7c19-es5.min.js 13.43 kB / 4.8 kB (gzip)
875.a4c54e0e-es5.min.js 12.55 kB / 4.83 kB (gzip)
session_replay-aggregate.4e039289-es5.min.js 12.53 kB / 4.6 kB (gzip)
page_view_event-aggregate.6ec43f65-es5.min.js 11.67 kB / 4.3 kB (gzip)
session_trace-aggregate.a193d3bf-es5.min.js 9.64 kB / 3.49 kB (gzip)
jserrors-aggregate.9b9e9ab4-es5.min.js 7.66 kB / 2.95 kB (gzip)
metrics-aggregate.35f401a9-es5.min.js 5.4 kB / 1.93 kB (gzip)
ajax-aggregate.f4dab427-es5.min.js 5.3 kB / 2.36 kB (gzip)
async-api.fc5fff8e-es5.min.js 3.07 kB / 1.5 kB (gzip)
page_action-aggregate.357c95d1-es5.min.js 2.84 kB / 1.23 kB (gzip)
session-manager.e66d8dc1-es5.min.js 2.41 kB / 1.06 kB (gzip)
lazy-feature-loader.56c21909-es5.min.js 1.14 kB / 507 B (gzip)

Built Assets

Asset Name Asset Size
recorder.b9a5173b-es5.min.js 166.76 kB / 52.35 kB (gzip)
nr-polyfills.min.js 52.15 kB / 17.95 kB (gzip)
compressor.39f21c15-es5.min.js 30.02 kB / 11.29 kB (gzip)
spa-aggregate.a2d4bd8d-es5.min.js 19.54 kB / 6.79 kB (gzip)
286.e45c50e1-es5.min.js 17.33 kB / 6.08 kB (gzip)
session_replay-aggregate.722970e4-es5.min.js 13.59 kB / 5.04 kB (gzip)
page_view_timing-aggregate.0b0f7c19-es5.min.js 13.43 kB / 4.8 kB (gzip)
875.6495c177-es5.min.js 13.31 kB / 4.91 kB (gzip)
page_view_event-aggregate.6ec43f65-es5.min.js 11.67 kB / 4.3 kB (gzip)
session_trace-aggregate.542471f3-es5.min.js 10.72 kB / 3.95 kB (gzip)
jserrors-aggregate.9b9e9ab4-es5.min.js 7.66 kB / 2.95 kB (gzip)
metrics-aggregate.35f401a9-es5.min.js 5.4 kB / 1.93 kB (gzip)
ajax-aggregate.f4dab427-es5.min.js 5.3 kB / 2.36 kB (gzip)
async-api.b00ab0c6-es5.min.js 4.14 kB / 2.01 kB (gzip)
page_action-aggregate.357c95d1-es5.min.js 2.84 kB / 1.23 kB (gzip)
session-manager.e66d8dc1-es5.min.js 2.41 kB / 1.06 kB (gzip)
lazy-feature-loader.56c21909-es5.min.js 1.14 kB / 507 B (gzip)

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2023

Codecov Report

Merging #544 (4fc0681) into main (b1c8792) will not change coverage.
The diff coverage is 20.00%.

@@           Coverage Diff           @@
##             main     #544   +/-   ##
=======================================
  Coverage   20.03%   20.03%           
=======================================
  Files         132      132           
  Lines        4347     4347           
  Branches     1109     1109           
=======================================
  Hits          871      871           
  Misses       2745     2745           
  Partials      731      731           
Impacted Files Coverage Δ
src/common/drain/drain.js 0.00% <0.00%> (ø)
src/common/wrap/wrap-fetch.js 17.39% <0.00%> (ø)
src/common/wrap/wrap-function.js 22.97% <0.00%> (ø)
src/loaders/agent.js 0.00% <0.00%> (ø)
src/common/wrap/wrap-promise.js 85.71% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bjfield
Copy link
Contributor Author

bjfield commented May 25, 2023

Manual verification:

  • No console errors appear for these nocompat builds of MooTools: 1.3.0, 1.3.1, 1.3.2, 1.4.1, 1.4.2, 1.4.3, 1.4.4, 1.4.5, 1.5.0, 1.5.1, 1.5.2, 1.6.0.
  • The reported error (Uncaught (in promise) TypeError: Failed to execute 'removeEventListener' on 'EventTarget': 2 arguments required, but only 0 present.) does not appear for any builds, although an Uncaught TypeError: t is not a function error does appear for builds with the compatibility layer. This other error does not break the web page, is not addressed by this issue, and is expected for all MooTools compat builds.

@bjfield bjfield merged commit f1e6336 into main May 26, 2023
@bjfield bjfield deleted the fix-array-from branch May 26, 2023 13:46
bjfield pushed a commit that referenced this pull request May 26, 2023
* Capture metrics for usage of MooTools and certain polyfills (#539) (903a7e1)

* Update agent internals in early preparation for new features (#532) (1ee675d)

* Address "configurable" warnings arising from user-agent module (#546) (7a7dace)

* Ensure runtime is preserved for late-configuration cases (#538) (229b8ed)

* Refactor usage of Array.from to address MooTools conflict (#544) (f1e6336)
metal-messiah pushed a commit that referenced this pull request Jun 5, 2023
metal-messiah pushed a commit that referenced this pull request Jun 5, 2023
* Capture metrics for usage of MooTools and certain polyfills (#539) (903a7e1)

* Update agent internals in early preparation for new features (#532) (1ee675d)

* Address "configurable" warnings arising from user-agent module (#546) (7a7dace)

* Ensure runtime is preserved for late-configuration cases (#538) (229b8ed)

* Refactor usage of Array.from to address MooTools conflict (#544) (f1e6336)
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.

4 participants