From c480c37c8e23d95fb976012e879376bb3a8bb275 Mon Sep 17 00:00:00 2001
From: Lachlan Campbell <lachlan@hackclub.com>
Date: Mon, 3 Feb 2020 14:55:14 -0500
Subject: [PATCH] Skip undefined attribute in Head (#9856)

* Fix #8655, skip rendering meta tags with undefined props

* Filter all tags, not just meta

* Only render defined props

* Remove filtering of undefined strings

Co-Authored-By: Tim Neutkens <tim@timneutkens.nl>

* Replace Object.entries

* Remove filtering code

* Simplify code

* Add test

* Add tests for undefined head prop value and tweak check

* Update to strip undefined prop values to match react

* Update head.js

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
Co-authored-by: Joe Haddad <timer150@gmail.com>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
---
 packages/next/client/head-manager.js                  | 3 +++
 test/integration/client-navigation/pages/head.js      | 3 +++
 test/integration/client-navigation/test/index.test.js | 9 +++++++++
 test/integration/client-navigation/test/rendering.js  | 8 ++++++++
 4 files changed, 23 insertions(+)

diff --git a/packages/next/client/head-manager.js b/packages/next/client/head-manager.js
index 865f50bc52a88..5eec36503c0ce 100644
--- a/packages/next/client/head-manager.js
+++ b/packages/next/client/head-manager.js
@@ -95,6 +95,9 @@ function reactElementToDOM({ type, props }) {
     if (!props.hasOwnProperty(p)) continue
     if (p === 'children' || p === 'dangerouslySetInnerHTML') continue
 
+    // we don't render undefined props to the DOM
+    if (props[p] === undefined) continue
+
     const attr = DOMAttributeNames[p] || p.toLowerCase()
     el.setAttribute(attr, props[p])
   }
diff --git a/test/integration/client-navigation/pages/head.js b/test/integration/client-navigation/pages/head.js
index 6b868ec0de995..ae698dc67e70e 100644
--- a/test/integration/client-navigation/pages/head.js
+++ b/test/integration/client-navigation/pages/head.js
@@ -16,6 +16,9 @@ export default () => (
 
       <meta content="my meta" />
 
+      {/* this will not render the content prop */}
+      <meta name="empty-content" content={undefined} />
+
       {/* allow duplicates for specific tags */}
       <meta property="article:tag" content="tag1" key="tag1key" />
       <meta property="article:tag" content="tag2" key="tag2key" />
diff --git a/test/integration/client-navigation/test/index.test.js b/test/integration/client-navigation/test/index.test.js
index c236e8bd249ac..81aab25d0b341 100644
--- a/test/integration/client-navigation/test/index.test.js
+++ b/test/integration/client-navigation/test/index.test.js
@@ -1097,6 +1097,15 @@ describe('Client Navigation', () => {
     await browser.close()
   })
 
+  it('should handle undefined prop in head client-side', async () => {
+    const browser = await webdriver(context.appPort, '/head')
+    const value = await browser.eval(
+      `document.querySelector('meta[name="empty-content"]').hasAttribute('content')`
+    )
+
+    expect(value).toBe(false)
+  })
+
   renderingSuite(
     (p, q) => renderViaHTTP(context.appPort, p, q),
     (p, q) => fetchViaHTTP(context.appPort, p, q)
diff --git a/test/integration/client-navigation/test/rendering.js b/test/integration/client-navigation/test/rendering.js
index b7679fbb45796..4bc7ef238d89d 100644
--- a/test/integration/client-navigation/test/rendering.js
+++ b/test/integration/client-navigation/test/rendering.js
@@ -17,6 +17,14 @@ export default function(render, fetch) {
       expect(html.includes('My component!')).toBeTruthy()
     })
 
+    it('should handle undefined prop in head server-side', async () => {
+      const html = await render('/head')
+      const $ = cheerio.load(html)
+      const value = 'content' in $('meta[name="empty-content"]').attr()
+
+      expect(value).toBe(false)
+    })
+
     test('renders with fragment syntax', async () => {
       const html = await render('/fragment-syntax')
       expect(html.includes('My component!')).toBeTruthy()