Skip to content

Commit

Permalink
Don’t overwrite classes during SSR when rendering fragments (#2173)
Browse files Browse the repository at this point in the history
* Refactor SSR test helpers

* Add SSR tests for transition

* Don’t overwrite classes during SSR when rendering fragments

* Update changelog
  • Loading branch information
thecrypticace authored Jan 12, 2023
1 parent 08c0837 commit aac78d5
Show file tree
Hide file tree
Showing 10 changed files with 205 additions and 113 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix `Tab` key with non focusable elements in `Popover.Panel` ([#2147](https://github.com/tailwindlabs/headlessui/pull/2147))
- Fix false positive warning when using `<Popover.Button />` in React 17 ([#2163](https://github.com/tailwindlabs/headlessui/pull/2163))
- Fix `failed to removeChild on Node` bug ([#2164](https://github.com/tailwindlabs/headlessui/pull/2164))
- Don’t overwrite classes during SSR when rendering fragments ([#2173](https://github.com/tailwindlabs/headlessui/pull/2173))

## [1.7.7] - 2022-12-16

Expand Down
80 changes: 6 additions & 74 deletions packages/@headlessui-react/src/components/tabs/tabs.ssr.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { RenderResult } from '@testing-library/react'
import { render, RenderOptions } from '@testing-library/react'
import React, { ReactElement } from 'react'
import { renderToString } from 'react-dom/server'
import React from 'react'
import { Tab } from './tabs'
import { env } from '../../utils/env'
import { renderSSR, renderHydrate } from '../../test-utils/ssr'

beforeAll(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any)
Expand Down Expand Up @@ -31,15 +28,15 @@ function Example({ defaultIndex = 0 }) {
describe('Rendering', () => {
describe('SSR', () => {
it('should be possible to server side render the first Tab and Panel', async () => {
let { contents } = await serverRender(<Example />)
let { contents } = await renderSSR(<Example />)

expect(contents).toContain(`Content 1`)
expect(contents).not.toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})

it('should be possible to server side render the defaultIndex Tab and Panel', async () => {
let { contents } = await serverRender(<Example defaultIndex={1} />)
let { contents } = await renderSSR(<Example defaultIndex={1} />)

expect(contents).not.toContain(`Content 1`)
expect(contents).toContain(`Content 2`)
Expand All @@ -51,84 +48,19 @@ describe('Rendering', () => {
// Skipping for now
xdescribe('Hydration', () => {
it('should be possible to server side render the first Tab and Panel', async () => {
const { contents } = await hydrateRender(<Example />)
const { contents } = await renderHydrate(<Example />)

expect(contents).toContain(`Content 1`)
expect(contents).not.toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})

it('should be possible to server side render the defaultIndex Tab and Panel', async () => {
const { contents } = await hydrateRender(<Example defaultIndex={1} />)
const { contents } = await renderHydrate(<Example defaultIndex={1} />)

expect(contents).not.toContain(`Content 1`)
expect(contents).toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})
})
})

type ServerRenderOptions = Omit<RenderOptions, 'queries'> & {
strict?: boolean
}

interface ServerRenderResult {
type: 'ssr' | 'hydrate'
contents: string
result: RenderResult
hydrate: () => Promise<ServerRenderResult>
}

async function serverRender(
ui: ReactElement,
options: ServerRenderOptions = {}
): Promise<ServerRenderResult> {
let container = document.createElement('div')
document.body.appendChild(container)
options = { ...options, container }

if (options.strict) {
options = {
...options,
wrapper({ children }) {
return <React.StrictMode>{children}</React.StrictMode>
},
}
}

env.set('server')
let contents = renderToString(ui)
let result = render(<div dangerouslySetInnerHTML={{ __html: contents }} />, options)

async function hydrate(): Promise<ServerRenderResult> {
// This hack-ish way of unmounting the server rendered content is necessary
// otherwise we won't actually end up testing the hydration code path properly.
// Probably because React hangs on to internal references on the DOM nodes
result.unmount()
container.innerHTML = contents

env.set('client')
let newResult = render(ui, {
...options,
hydrate: true,
})

return {
type: 'hydrate',
contents: container.innerHTML,
result: newResult,
hydrate,
}
}

return {
type: 'ssr',
contents,
result,
hydrate,
}
}

async function hydrateRender(el: ReactElement, options: ServerRenderOptions = {}) {
return serverRender(el, options).then((r) => r.hydrate())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import React, { Fragment } from 'react'
import { Transition } from './transition'
import { renderSSR } from '../../test-utils/ssr'

beforeAll(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any)
jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any)
})

describe('Rendering', () => {
describe('SSR', () => {
it('should not overwrite className of children when as=Fragment', async () => {
await renderSSR(
<Transition
as={Fragment}
show={true}
appear={true}
enter="enter"
enterFrom="enter-from"
enterTo="enter-to"
>
<div className="inner"></div>
</Transition>
)

let div = document.querySelector('.inner')

expect(div).not.toBeNull()
expect(div?.className).toBe('inner enter enter-from')
})
})
})
70 changes: 70 additions & 0 deletions packages/@headlessui-react/src/test-utils/ssr.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { RenderResult } from '@testing-library/react'
import { render, RenderOptions } from '@testing-library/react'
import React, { ReactElement } from 'react'
import { renderToString } from 'react-dom/server'
import { env } from '../utils/env'

type ServerRenderOptions = Omit<RenderOptions, 'queries'> & {
strict?: boolean
}

interface ServerRenderResult {
type: 'ssr' | 'hydrate'
contents: string
result: RenderResult
hydrate: () => Promise<ServerRenderResult>
}

export async function renderSSR(
ui: ReactElement,
options: ServerRenderOptions = {}
): Promise<ServerRenderResult> {
let container = document.createElement('div')
document.body.appendChild(container)
options = { ...options, container }

if (options.strict) {
options = {
...options,
wrapper({ children }) {
return <React.StrictMode>{children}</React.StrictMode>
},
}
}

env.set('server')
let contents = renderToString(ui)
let result = render(<div dangerouslySetInnerHTML={{ __html: contents }} />, options)

async function hydrate(): Promise<ServerRenderResult> {
// This hack-ish way of unmounting the server rendered content is necessary
// otherwise we won't actually end up testing the hydration code path properly.
// Probably because React hangs on to internal references on the DOM nodes
result.unmount()
container.innerHTML = contents

env.set('client')
let newResult = render(ui, {
...options,
hydrate: true,
})

return {
type: 'hydrate',
contents: container.innerHTML,
result: newResult,
hydrate,
}
}

return {
type: 'ssr',
contents,
result,
hydrate,
}
}

export async function renderHydrate(el: ReactElement, options: ServerRenderOptions = {}) {
return renderSSR(el, options).then((r) => r.hydrate())
}
9 changes: 8 additions & 1 deletion packages/@headlessui-react/src/utils/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
ReactElement,
} from 'react'
import { Props, XOR, __, Expand } from '../types'
import { classNames } from './class-names'
import { env } from './env'
import { match } from './match'

export enum Features {
Expand Down Expand Up @@ -168,6 +170,10 @@ function _render<TTag extends ElementType, TSlot>(
)
}

// Merge class name prop in SSR
let newClassName = classNames(resolvedChildren.props?.className, rest.className)
let classNameProps = newClassName ? { className: newClassName } : {}

return cloneElement(
resolvedChildren,
Object.assign(
Expand All @@ -176,7 +182,8 @@ function _render<TTag extends ElementType, TSlot>(
mergeProps(resolvedChildren.props, compact(omit(rest, ['ref']))),
dataAttributes,
refRelatedProps,
mergeRefs((resolvedChildren as any).ref, refRelatedProps.ref)
mergeRefs((resolvedChildren as any).ref, refRelatedProps.ref),
classNameProps
)
)
}
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Ensure `disabled="false"` is not incorrectly passed to the underlying DOM Node ([#2138](https://github.com/tailwindlabs/headlessui/pull/2138))
- Fix arrow key handling in `Tab` (after DOM order changes) ([#2145](https://github.com/tailwindlabs/headlessui/pull/2145))
- Fix `Tab` key with non focusable elements in `Popover.Panel` ([#2147](https://github.com/tailwindlabs/headlessui/pull/2147))
- Don’t overwrite classes during SSR when rendering fragments ([#2173](https://github.com/tailwindlabs/headlessui/pull/2173))

## [1.7.7] - 2022-12-16

Expand Down
41 changes: 6 additions & 35 deletions packages/@headlessui-vue/src/components/tabs/tabs.ssr.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { createApp, createSSRApp, defineComponent, h } from 'vue'
import { renderToString } from 'vue/server-renderer'
import { defineComponent } from 'vue'
import { TabGroup, TabList, Tab, TabPanels, TabPanel } from './tabs'
import { html } from '../../test-utils/html'
import { render } from '../../test-utils/vue-testing-library'
import { env } from '../../utils/env'
import { renderHydrate, renderSSR } from '../../test-utils/ssr'

jest.mock('../../hooks/use-id')

Expand Down Expand Up @@ -36,15 +34,15 @@ let Example = defineComponent({
describe('Rendering', () => {
describe('SSR', () => {
it('should be possible to server side render the first Tab and Panel', async () => {
let { contents } = await serverRender(Example)
let { contents } = await renderSSR(Example)

expect(contents).toContain(`Content 1`)
expect(contents).not.toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})

it('should be possible to server side render the defaultIndex Tab and Panel', async () => {
let { contents } = await serverRender(Example, { defaultIndex: 1 })
let { contents } = await renderSSR(Example, { defaultIndex: 1 })

expect(contents).not.toContain(`Content 1`)
expect(contents).toContain(`Content 2`)
Expand All @@ -54,46 +52,19 @@ describe('Rendering', () => {

describe('Hydration', () => {
it('should be possible to server side render the first Tab and Panel', async () => {
let { contents } = await hydrateRender(Example)
let { contents } = await renderHydrate(Example)

expect(contents).toContain(`Content 1`)
expect(contents).not.toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})

it('should be possible to server side render the defaultIndex Tab and Panel', async () => {
let { contents } = await hydrateRender(Example, { defaultIndex: 1 })
let { contents } = await renderHydrate(Example, { defaultIndex: 1 })

expect(contents).not.toContain(`Content 1`)
expect(contents).toContain(`Content 2`)
expect(contents).not.toContain(`Content 3`)
})
})
})

async function serverRender(component: any, rootProps: any = {}) {
let container = document.createElement('div')
document.body.appendChild(container)

// Render on the server
env.set('server')
let app = createSSRApp(component, rootProps)
let contents = await renderToString(app)
container.innerHTML = contents

return {
contents,
hydrate() {
let app = createApp(component, rootProps)
app.mount(container)

return {
contents: container.innerHTML,
}
},
}
}

async function hydrateRender(component: any, rootProps: any = {}) {
return serverRender(component, rootProps).then(({ hydrate }) => hydrate())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import * as Transition from './transition'
import { renderSSR } from '../../test-utils/ssr'
import { defineComponent } from 'vue'
import { html } from '../../test-utils/html'

beforeAll(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(setImmediate as any)
jest.spyOn(window, 'cancelAnimationFrame').mockImplementation(clearImmediate as any)
})

describe('Rendering', () => {
describe('SSR', () => {
it('should not overwrite className of children when as=Fragment', async () => {
await renderSSR(
defineComponent({
components: Transition,
template: html`
<TransitionRoot
as="template"
:show="true"
:appear="true"
enter="enter"
enterFrom="enter-from"
enterTo="enter-to"
>
<div class="inner"></div>
</TransitionRoot>
`,
})
)

let div = document.querySelector('.inner')

expect(div).not.toBeNull()
expect(div?.className).toBe('inner enter enter-from')
})
})
})
Loading

0 comments on commit aac78d5

Please sign in to comment.