Bind sendBeacon to navigator #26601

timneutkens · 2021-06-25T12:20:59Z

Also ensures that even if the navigator.sendBeacon fails the fetch fallback is used.

Fixes #23856. This is likely, as no reproduction was provided it was not possible to verify if it actually fixes the issue.

Bug

Related issues linked using fixes #number
Integration tests added

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
Related issues linked using fixes #number
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.

Documentation / Examples

Make sure the linting passes

Also ensures that even if the navigator.sendBeacon fails the fetch fallback is used. Fixes vercel#23856. This is likely, as no reproduction was provided it was not possible to verify if it actually fixes the issue.

packages/next/client/performance-relayer.ts

pvdz

It took me a few reads to realize that calling send even if it may not be a callable is an expected situation, intended to be caught by the trap.

I probably would write this a little different, but it's not necessarily wrong.

Make sure you're okay with the potential of calling fallbackSend twice (in case the first call inside the try fails).

packages/next/client/performance-relayer.ts

Co-authored-by: Steven <[email protected]>

ijjk · 2021-07-12T13:25:24Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
buildDuration	11.7s	12.4s	⚠️ +712ms
buildDurationCached	2.8s	2.8s	⚠️ +5ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +1.23 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.909	1.984	⚠️ +0.07
/ avg req/sec	1309.28	1260.16	⚠️ -49.12
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.082	1.112	⚠️ +0.03
/error-in-render avg req/sec	2310.5	2247.67	⚠️ -62.83

Client Bundles (main, webpack, commons)

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
359.HASH.js gzip	3.09 kB	3.09 kB	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	20.6 kB	20.6 kB	✓
webpack-HASH.js gzip	1.49 kB	1.49 kB	✓
Overall change	67.2 kB	67.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
_app-HASH.js gzip	803 B	803 B	✓
_error-HASH.js gzip	3.18 kB	3.18 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	329 B	329 B	✓
hooks-HASH.js gzip	903 B	903 B	✓
image-HASH.js gzip	5.69 kB	5.69 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	1.66 kB	1.66 kB	✓
routerDirect..HASH.js gzip	319 B	319 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	14.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
_buildManifest.js gzip	419 B	419 B	✓
Overall change	419 B	419 B	✓

Rendered Page Sizes

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	544 B	✓
withRouter.html gzip	525 B	525 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for main-HASH.js

@@ -2133,7 +2133,7 @@
         // message during the build (`next build`).
 
         if (false) {
-          var vitalsUrl, blob, body;
+          var send, vitalsUrl, blob, body, fallbackSend;
         }
       }

Diff for index.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-fe313358475ae5ea15f4.js"
+      src="/_next/static/chunks/main-4f15dc261f5ce5c05417.js"
       defer=""
     ></script>
     <script

Diff for link.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-fe313358475ae5ea15f4.js"
+      src="/_next/static/chunks/main-4f15dc261f5ce5c05417.js"
       defer=""
     ></script>
     <script

Diff for withRouter.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-fe313358475ae5ea15f4.js"
+      src="/_next/static/chunks/main-4f15dc261f5ce5c05417.js"
       defer=""
     ></script>
     <script

Webpack 4 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
buildDuration	9.5s	9.3s	-206ms
buildDurationCached	4s	3.6s	-352ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +1.23 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.924	1.908	-0.02
/ avg req/sec	1299.2	1310.04	+10.84
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.132	1.155	⚠️ +0.02
/error-in-render avg req/sec	2209.16	2163.61	⚠️ -45.55

Client Bundles (main, webpack, commons)

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
14.HASH.js gzip	3.11 kB	3.11 kB	✓
677f882d2ed8..HASH.js gzip	13.9 kB	13.9 kB	✓
framework.HASH.js gzip	41.8 kB	41.8 kB	✓
main-HASH.js gzip	7.81 kB	7.81 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.8 kB	67.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
_app-HASH.js gzip	791 B	791 B	✓
_error-HASH.js gzip	3.83 kB	3.83 kB	✓
amp-HASH.js gzip	531 B	531 B	✓
css-HASH.js gzip	333 B	333 B	✓
hooks-HASH.js gzip	910 B	910 B	✓
index-HASH.js gzip	230 B	230 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	297 B	297 B	✓
withRouter-HASH.js gzip	293 B	293 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	8.98 kB	8.98 kB	✓

Client Build Manifests

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
_buildManifest.js gzip	418 B	418 B	✓
Overall change	418 B	418 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	timneutkens/next.js fix/sendbeacon-issue	Change
index.html gzip	575 B	576 B	⚠️ +1 B
link.html gzip	587 B	587 B	✓
withRouter.html gzip	570 B	570 B	✓
Overall change	1.73 kB	1.73 kB	⚠️ +1 B

Diffs

Diff for main-HASH.js

@@ -1623,7 +1623,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
         // message during the build (`next build`).
 
         if (false) {
-          var vitalsUrl, blob, body;
+          var send, vitalsUrl, blob, body, fallbackSend;
         }
       }

Diff for index.html

@@ -23,7 +23,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-30f6dfd95ffbda38567f.js"
+      src="/_next/static/chunks/main-a89c0cf085ce34d3395c.js"
       defer=""
     ></script>
     <script

Diff for link.html

@@ -23,7 +23,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-30f6dfd95ffbda38567f.js"
+      src="/_next/static/chunks/main-a89c0cf085ce34d3395c.js"
       defer=""
     ></script>
     <script

Diff for withRouter.html

@@ -23,7 +23,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-30f6dfd95ffbda38567f.js"
+      src="/_next/static/chunks/main-a89c0cf085ce34d3395c.js"
       defer=""
     ></script>
     <script

Commit: e08ce04

Also ensures that even if the navigator.sendBeacon fails the fetch fallback is used. Fixes vercel#23856. This is likely, as no reproduction was provided it was not possible to verify if it actually fixes the issue. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

Bind sendBeacon to navigator

eee2ba5

Also ensures that even if the navigator.sendBeacon fails the fetch fallback is used. Fixes vercel#23856. This is likely, as no reproduction was provided it was not possible to verify if it actually fixes the issue.

timneutkens requested review from divmain, ijjk, lfades and shuding as code owners June 25, 2021 12:20

ijjk added the type: next label Jun 25, 2021

This comment has been minimized.

Sign in to view

ijjk reviewed Jun 25, 2021

View reviewed changes

packages/next/client/performance-relayer.ts Show resolved Hide resolved

pvdz previously approved these changes Jun 25, 2021

View reviewed changes

timneutkens added 2 commits July 10, 2021 22:35

Check if send exists and catch fetch error

c2fd400

Add comment as to why the bind is needed

43131b4

timneutkens dismissed pvdz’s stale review via 43131b4 July 10, 2021 20:38

timneutkens requested review from huozhi, padmaia and styfle as code owners July 10, 2021 20:38

ijjk added the created-by: Next.js team PRs by the Next.js team. label Jul 10, 2021

Add comment about console.error usage

c042295

This comment has been minimized.

Sign in to view

ijjk previously approved these changes Jul 11, 2021

View reviewed changes

Merge branch 'canary' into fix/sendbeacon-issue

c4ef9d6

This comment has been minimized.

Sign in to view

styfle reviewed Jul 11, 2021

View reviewed changes

packages/next/client/performance-relayer.ts Outdated Show resolved Hide resolved

Update packages/next/client/performance-relayer.ts

d74e75c

Co-authored-by: Steven <[email protected]>

timneutkens dismissed ijjk’s stale review via d74e75c July 12, 2021 10:08

Add why comment

4f3a500

styfle approved these changes Jul 12, 2021

View reviewed changes

Merge branch 'canary' into fix/sendbeacon-issue

4b404f7

ijjk approved these changes Jul 12, 2021

View reviewed changes

Merge branch 'canary' into fix/sendbeacon-issue

e08ce04

This comment has been minimized.

Sign in to view

kodiakhq bot merged commit 1bcc923 into vercel:canary Jul 12, 2021

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bind sendBeacon to navigator #26601

Bind sendBeacon to navigator #26601

timneutkens commented Jun 25, 2021

This comment has been minimized.

pvdz left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jul 12, 2021

Diffs

Diffs

Bind sendBeacon to navigator #26601

Bind sendBeacon to navigator #26601

Conversation

timneutkens commented Jun 25, 2021

Bug

Feature

Documentation / Examples

This comment has been minimized.

pvdz left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jul 12, 2021

Stats from current PR

Diffs

Diffs