Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don’t overwrite classes during SSR when rendering fragments #2173

Merged
merged 4 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'
thecrypticace marked this conversation as resolved.
Show resolved Hide resolved

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', () => {
thecrypticace marked this conversation as resolved.
Show resolved Hide resolved
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