fixes #31060: NullReferenceException #31061

furcan · 2021-11-05T23:14:56Z

Fixes NullReferenceException: "packages/next/client/head-manager.ts" #31060

Related Issue: #31060

furcan · 2021-11-08T06:25:00Z

Hi,
Any comments?
Thanks in advance.

timneutkens

Could you add an integration test for this?

furcan · 2021-11-08T14:04:30Z

Hi @timneutkens ,

My solution just focuses to avoid the possible null reference exceptions. I have to understand these functions behaviors and purposes first to add the tests, and so, I am not able to do this right now.

So, maybe you can consider refactoring this package from scratch. Or, my temporary solution can be accepted to avoid errors that have been mentioned in the linked issue.

_{P.S.: This issue is happening with the next export as I have mentioned in the linked issue.}

Thanks in advance,
Furkan.

packages/next/client/head-manager.ts

styfle

I agree we need a test.

I suspect that the actual failure is that the meta element is missing from the DOM.

Try this on your page:

document.querySelector('meta[name=next-head-count]')

packages/next/client/head-manager.ts

furcan · 2021-11-08T16:54:16Z

Hi @styfle ,

I already checked the 'meta[name=next-head-count]' meta element before creating the issue and PR.
And also, the error would be on line 61(head-manager.ts) if this element would be missing.

I already have that meta element in my application and it has 46 content.

I think these null checks in this PR (with your additional suggestions) will be solved the issue.

In addition, there is a ScreenShot below that might be helpful. (A console demonstration)

Edit:
Without null check: (Same as the issue)

FYI: @styfle

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

ijjk · 2021-11-08T17:16:10Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
buildDuration	24.6s	24.4s	-222ms
buildDurationCached	4.8s	4.8s	⚠️ +11ms
nodeModulesSize	332 MB	332 MB	⚠️ +289 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
/ failed reqs	0	0	✓
/ total time (seconds)	4.112	4.173	⚠️ +0.06
/ avg req/sec	608.04	599.13	⚠️ -8.91
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.048	2.077	⚠️ +0.03
/error-in-render avg req/sec	1220.58	1203.9	⚠️ -16.68

Client Bundles (main, webpack, commons) Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	28 kB	28.1 kB	⚠️ +21 B
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	71.9 kB	71.9 kB	⚠️ +21 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	furcan/next.js patch-2	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	furcan/next.js patch-2	Change
_app-HASH.js gzip	1.23 kB	1.23 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	635 B	635 B	✓
image-HASH.js gzip	4.44 kB	4.44 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	1.87 kB	1.87 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
334f979574ae..6f4.css gzip	106 B	106 B	✓
Overall change	13.1 kB	13.1 kB	✓

Client Build Manifests

	vercel/next.js canary	furcan/next.js patch-2	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
index.html gzip	521 B	522 B	⚠️ +1 B
link.html gzip	534 B	535 B	⚠️ +1 B
withRouter.html gzip	515 B	516 B	⚠️ +1 B
Overall change	1.57 kB	1.57 kB	⚠️ +3 B

Diffs

Diff for main-HASH.js

@@ -116,7 +116,11 @@
         for (
           var i = 0, j = headCountEl.previousElementSibling;
           i < headCount;
-          i++, j = j.previousElementSibling
+          i++,
+            j =
+              (j === null || j === void 0
+                ? void 0
+                : j.previousElementSibling) || null
         ) {
           var ref;
           if (
@@ -142,7 +146,10 @@
             return true;
           });
         oldTags.forEach(function(t) {
-          return t.parentNode.removeChild(t);
+          var ref;
+          return (ref = t.parentNode) === null || ref === void 0
+            ? void 0
+            : ref.removeChild(t);
         });
         newTags.forEach(function(t) {
           return headEl.insertBefore(t, headCountEl);

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
buildDuration	27.3s	26.9s	-341ms
buildDurationCached	4.7s	4.8s	⚠️ +39ms
nodeModulesSize	332 MB	332 MB	⚠️ +289 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	furcan/next.js patch-2	Change
/ failed reqs	0	0	✓
/ total time (seconds)	4.078	4.084	⚠️ +0.01
/ avg req/sec	613.07	612.16	⚠️ -0.91
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.103	2.094	-0.01
/error-in-render avg req/sec	1188.9	1193.89	+4.99

Client Bundles (main, webpack, commons) Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	28.2 kB	28.2 kB	⚠️ +13 B
webpack-HASH.js gzip	1.43 kB	1.43 kB	✓
Overall change	72.1 kB	72.1 kB	⚠️ +13 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	furcan/next.js patch-2	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	furcan/next.js patch-2	Change
_app-HASH.js gzip	1.22 kB	1.22 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.38 kB	2.38 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	622 B	622 B	✓
image-HASH.js gzip	4.46 kB	4.46 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	1.91 kB	1.91 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
334f979574ae..6f4.css gzip	106 B	106 B	✓
Overall change	13.1 kB	13.1 kB	✓

Client Build Manifests

	vercel/next.js canary	furcan/next.js patch-2	Change
_buildManifest.js gzip	460 B	460 B	✓
Overall change	460 B	460 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	furcan/next.js patch-2	Change
index.html gzip	521 B	522 B	⚠️ +1 B
link.html gzip	534 B	535 B	⚠️ +1 B
withRouter.html gzip	515 B	516 B	⚠️ +1 B
Overall change	1.57 kB	1.57 kB	⚠️ +3 B

Diffs

Diff for main-HASH.js

@@ -116,7 +116,11 @@
         for (
           var i = 0, j = headCountEl.previousElementSibling;
           i < headCount;
-          i++, j = j.previousElementSibling
+          i++,
+            j =
+              (j === null || j === void 0
+                ? void 0
+                : j.previousElementSibling) || null
         ) {
           var ref;
           if (
@@ -142,7 +146,10 @@
             return true;
           });
         oldTags.forEach(function(t) {
-          return t.parentNode.removeChild(t);
+          var ref;
+          return (ref = t.parentNode) === null || ref === void 0
+            ? void 0
+            : ref.removeChild(t);
         });
         newTags.forEach(function(t) {
           return headEl.insertBefore(t, headCountEl);

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Commit: c15475b

styfle · 2021-11-08T17:53:09Z

I think this is safe to merge, even without tests, since its unlikely to cause any harm and probably wont regress because TS will complain if someone tries to remove the ? in a future PR 👍

Safe without tests

fix(#31060): NullReferenceException

2dd2528

Related Issue: #31060

furcan requested review from huozhi, ijjk, shuding and timneutkens as code owners November 5, 2021 23:14

ijjk added the type: next label Nov 5, 2021

furcan changed the title ~~fix(#31060): NullReferenceException~~ fixes #31060: NullReferenceException Nov 5, 2021

Type 'undefined' is not assignable to type 'Element | null'

de9d4c6

This comment has been minimized.

Sign in to view

timneutkens previously requested changes Nov 8, 2021

View reviewed changes

styfle reviewed Nov 8, 2021

View reviewed changes

packages/next/client/head-manager.ts Show resolved Hide resolved

styfle reviewed Nov 8, 2021

View reviewed changes

packages/next/client/head-manager.ts Outdated Show resolved Hide resolved

packages/next/client/head-manager.ts Outdated Show resolved Hide resolved

furcan and others added 2 commits November 8, 2021 19:54

Update packages/next/client/head-manager.ts

439549f

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

Update packages/next/client/head-manager.ts

c15475b

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

styfle approved these changes Nov 8, 2021

View reviewed changes

styfle requested a review from timneutkens November 8, 2021 17:08

styfle merged commit 58755e6 into vercel:canary Nov 8, 2021

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #31060: NullReferenceException #31061

fixes #31060: NullReferenceException #31061

furcan commented Nov 5, 2021 •

edited

Loading

This comment has been minimized.

furcan commented Nov 8, 2021

timneutkens left a comment

furcan commented Nov 8, 2021

styfle left a comment •

edited

Loading

furcan commented Nov 8, 2021 •

edited

Loading

ijjk commented Nov 8, 2021

Diffs

Diffs

styfle commented Nov 8, 2021

fixes #31060: NullReferenceException #31061

fixes #31060: NullReferenceException #31061

Conversation

furcan commented Nov 5, 2021 • edited Loading

This comment has been minimized.

furcan commented Nov 8, 2021

timneutkens left a comment

Choose a reason for hiding this comment

furcan commented Nov 8, 2021

styfle left a comment • edited Loading

Choose a reason for hiding this comment

furcan commented Nov 8, 2021 • edited Loading

ijjk commented Nov 8, 2021

Stats from current PR

Diffs

Diffs

styfle commented Nov 8, 2021

furcan commented Nov 5, 2021 •

edited

Loading

styfle left a comment •

edited

Loading

furcan commented Nov 8, 2021 •

edited

Loading