Skip to content

Commit

Permalink
Update to strip undefined prop values to match react
Browse files Browse the repository at this point in the history
  • Loading branch information
ijjk committed Feb 3, 2020
1 parent 6ed862d commit dff4aa5
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 19 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
15 changes: 2 additions & 13 deletions packages/next/next-server/lib/head.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,19 +140,8 @@ function reduceComponents(
.filter(unique())
.reverse()
.map((c: React.ReactElement<any>, i: number) => {
const cleanProps: { [key: string]: string } = {}

Object.keys(c.props).forEach(key => {
let val = c.props[key]

// TODO: do we want to normalize null/true/false here also?
if (val === undefined) {
val = ''
}
cleanProps[key] = val
})

return React.cloneElement(c, { key: c.key || i, ...cleanProps })
const key = c.key || i
return React.cloneElement(c, { key })
})
}

Expand Down
8 changes: 4 additions & 4 deletions test/integration/client-navigation/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,11 +1099,11 @@ describe('Client Navigation', () => {

it('should handle undefined prop in head client-side', async () => {
const browser = await webdriver(context.appPort, '/head')
const value = await browser
.elementByCss('meta[name="empty-content"]')
.getAttribute('content')
const value = await browser.eval(
`document.querySelector('meta[name="empty-content"]').hasAttribute('content')`
)

expect(value).toBe('')
expect(value).toBe(false)
})

renderingSuite(
Expand Down
4 changes: 2 additions & 2 deletions test/integration/client-navigation/test/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ export default function(render, fetch) {
it('should handle undefined prop in head server-side', async () => {
const html = await render('/head')
const $ = cheerio.load(html)
const value = $('meta[name="empty-content"]').attr('content')
const value = 'content' in $('meta[name="empty-content"]').attr()

expect(value).toBe('')
expect(value).toBe(false)
})

test('renders with fragment syntax', async () => {
Expand Down

0 comments on commit dff4aa5

Please sign in to comment.