Skip to content

Commit

Permalink
Skip undefined attribute in Head (#9856)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
4 people authored Feb 3, 2020
1 parent 9a8c310 commit c480c37
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 0 deletions.
3 changes: 3 additions & 0 deletions packages/next/client/head-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
Expand Down
3 changes: 3 additions & 0 deletions test/integration/client-navigation/pages/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
9 changes: 9 additions & 0 deletions test/integration/client-navigation/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions test/integration/client-navigation/test/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit c480c37

Please sign in to comment.