From fc14675ee107f1492d4cf0866f8250fe43809f95 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 9 Jan 2023 15:17:26 -0500 Subject: [PATCH 1/4] Refactor SSR test helpers --- .../src/components/tabs/tabs.ssr.test.tsx | 80 ++----------------- .../@headlessui-react/src/test-utils/ssr.tsx | 70 ++++++++++++++++ .../src/components/tabs/tabs.ssr.test.ts | 41 ++-------- .../@headlessui-vue/src/test-utils/ssr.ts | 30 +++++++ 4 files changed, 112 insertions(+), 109 deletions(-) create mode 100644 packages/@headlessui-react/src/test-utils/ssr.tsx create mode 100644 packages/@headlessui-vue/src/test-utils/ssr.ts diff --git a/packages/@headlessui-react/src/components/tabs/tabs.ssr.test.tsx b/packages/@headlessui-react/src/components/tabs/tabs.ssr.test.tsx index 31736b6a0b..c28834ee50 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.ssr.test.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.ssr.test.tsx @@ -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) @@ -31,7 +28,7 @@ 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() + let { contents } = await renderSSR() expect(contents).toContain(`Content 1`) expect(contents).not.toContain(`Content 2`) @@ -39,7 +36,7 @@ describe('Rendering', () => { }) it('should be possible to server side render the defaultIndex Tab and Panel', async () => { - let { contents } = await serverRender() + let { contents } = await renderSSR() expect(contents).not.toContain(`Content 1`) expect(contents).toContain(`Content 2`) @@ -51,7 +48,7 @@ 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() + const { contents } = await renderHydrate() expect(contents).toContain(`Content 1`) expect(contents).not.toContain(`Content 2`) @@ -59,7 +56,7 @@ describe('Rendering', () => { }) it('should be possible to server side render the defaultIndex Tab and Panel', async () => { - const { contents } = await hydrateRender() + const { contents } = await renderHydrate() expect(contents).not.toContain(`Content 1`) expect(contents).toContain(`Content 2`) @@ -67,68 +64,3 @@ describe('Rendering', () => { }) }) }) - -type ServerRenderOptions = Omit & { - strict?: boolean -} - -interface ServerRenderResult { - type: 'ssr' | 'hydrate' - contents: string - result: RenderResult - hydrate: () => Promise -} - -async function serverRender( - ui: ReactElement, - options: ServerRenderOptions = {} -): Promise { - let container = document.createElement('div') - document.body.appendChild(container) - options = { ...options, container } - - if (options.strict) { - options = { - ...options, - wrapper({ children }) { - return {children} - }, - } - } - - env.set('server') - let contents = renderToString(ui) - let result = render(
, options) - - async function hydrate(): Promise { - // 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()) -} diff --git a/packages/@headlessui-react/src/test-utils/ssr.tsx b/packages/@headlessui-react/src/test-utils/ssr.tsx new file mode 100644 index 0000000000..2ec14c98b7 --- /dev/null +++ b/packages/@headlessui-react/src/test-utils/ssr.tsx @@ -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 & { + strict?: boolean +} + +interface ServerRenderResult { + type: 'ssr' | 'hydrate' + contents: string + result: RenderResult + hydrate: () => Promise +} + +export async function renderSSR( + ui: ReactElement, + options: ServerRenderOptions = {} +): Promise { + let container = document.createElement('div') + document.body.appendChild(container) + options = { ...options, container } + + if (options.strict) { + options = { + ...options, + wrapper({ children }) { + return {children} + }, + } + } + + env.set('server') + let contents = renderToString(ui) + let result = render(
, options) + + async function hydrate(): Promise { + // 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()) +} diff --git a/packages/@headlessui-vue/src/components/tabs/tabs.ssr.test.ts b/packages/@headlessui-vue/src/components/tabs/tabs.ssr.test.ts index eaa9648c18..144436364c 100644 --- a/packages/@headlessui-vue/src/components/tabs/tabs.ssr.test.ts +++ b/packages/@headlessui-vue/src/components/tabs/tabs.ssr.test.ts @@ -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') @@ -36,7 +34,7 @@ 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`) @@ -44,7 +42,7 @@ describe('Rendering', () => { }) 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`) @@ -54,7 +52,7 @@ 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`) @@ -62,7 +60,7 @@ describe('Rendering', () => { }) 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`) @@ -70,30 +68,3 @@ describe('Rendering', () => { }) }) }) - -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()) -} diff --git a/packages/@headlessui-vue/src/test-utils/ssr.ts b/packages/@headlessui-vue/src/test-utils/ssr.ts new file mode 100644 index 0000000000..0ec8734aae --- /dev/null +++ b/packages/@headlessui-vue/src/test-utils/ssr.ts @@ -0,0 +1,30 @@ +import { createApp, createSSRApp } from 'vue' +import { renderToString } from 'vue/server-renderer' +import { env } from '../utils/env' + +export async function renderSSR(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, + } + }, + } +} + +export async function renderHydrate(component: any, rootProps: any = {}) { + return renderSSR(component, rootProps).then(({ hydrate }) => hydrate()) +} From e3071f7d9b900ddf8af19a1e08103fdb5db30783 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 9 Jan 2023 15:47:34 -0500 Subject: [PATCH 2/4] Add SSR tests for transition --- .../transitions/transition.ssr.test.tsx | 32 ++++++++++++++++ .../transitions/transition.ssr.test.ts | 38 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 packages/@headlessui-react/src/components/transitions/transition.ssr.test.tsx create mode 100644 packages/@headlessui-vue/src/components/transitions/transition.ssr.test.ts diff --git a/packages/@headlessui-react/src/components/transitions/transition.ssr.test.tsx b/packages/@headlessui-react/src/components/transitions/transition.ssr.test.tsx new file mode 100644 index 0000000000..64a6108abd --- /dev/null +++ b/packages/@headlessui-react/src/components/transitions/transition.ssr.test.tsx @@ -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( + +
+ + ) + + let div = document.querySelector('.inner') + + expect(div).not.toBeNull() + expect(div?.className).toBe('inner enter enter-from') + }) + }) +}) diff --git a/packages/@headlessui-vue/src/components/transitions/transition.ssr.test.ts b/packages/@headlessui-vue/src/components/transitions/transition.ssr.test.ts new file mode 100644 index 0000000000..a43053eb04 --- /dev/null +++ b/packages/@headlessui-vue/src/components/transitions/transition.ssr.test.ts @@ -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` + +
+ + `, + }) + ) + + let div = document.querySelector('.inner') + + expect(div).not.toBeNull() + expect(div?.className).toBe('inner enter enter-from') + }) + }) +}) From 18bc50bc432e28fcf8c9b6ea5150436408bd0f20 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 9 Jan 2023 15:47:02 -0500 Subject: [PATCH 3/4] =?UTF-8?q?Don=E2=80=99t=20overwrite=20classes=20durin?= =?UTF-8?q?g=20SSR=20when=20rendering=20fragments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/@headlessui-react/src/utils/render.ts | 9 ++++++++- .../src/components/transitions/transition.ts | 16 +++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/src/utils/render.ts b/packages/@headlessui-react/src/utils/render.ts index 2978f46f2e..a0c8663308 100644 --- a/packages/@headlessui-react/src/utils/render.ts +++ b/packages/@headlessui-react/src/utils/render.ts @@ -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 { @@ -168,6 +170,10 @@ function _render( ) } + // Merge class name prop in SSR + let newClassName = classNames(resolvedChildren.props?.className, rest.className) + let classNameProps = newClassName ? { className: newClassName } : {} + return cloneElement( resolvedChildren, Object.assign( @@ -176,7 +182,8 @@ function _render( mergeProps(resolvedChildren.props, compact(omit(rest, ['ref']))), dataAttributes, refRelatedProps, - mergeRefs((resolvedChildren as any).ref, refRelatedProps.ref) + mergeRefs((resolvedChildren as any).ref, refRelatedProps.ref), + classNameProps ) ) } diff --git a/packages/@headlessui-vue/src/components/transitions/transition.ts b/packages/@headlessui-vue/src/components/transitions/transition.ts index b7b7914066..527c250707 100644 --- a/packages/@headlessui-vue/src/components/transitions/transition.ts +++ b/packages/@headlessui-vue/src/components/transitions/transition.ts @@ -14,10 +14,12 @@ import { InjectionKey, Ref, ConcreteComponent, + normalizeClass, } from 'vue' import { useId } from '../../hooks/use-id' import { match } from '../../utils/match' +import { env } from '../../utils/env' import { Features, omit, render, RenderStrategy } from '../../utils/render' import { Reason, transition } from './utils/transition' @@ -312,8 +314,8 @@ export let TransitionChild = defineComponent({ return () => { let { - appear, - show, + appear: _appear, + show: _show, // Class names enter, @@ -327,7 +329,15 @@ export let TransitionChild = defineComponent({ } = props let ourProps = { ref: container } - let theirProps = rest + let theirProps = { + ...rest, + ...(appear && show && env.isServer + ? { + // Already apply the `enter` and `enterFrom` on the server if required + class: normalizeClass([rest.class, ...enterClasses, ...enterFromClasses]), + } + : {}), + } return render({ theirProps, From 1d981fafe21d6c43d51e161047efeee08fae6358 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Thu, 12 Jan 2023 11:15:38 -0500 Subject: [PATCH 4/4] Update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + packages/@headlessui-vue/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 07c20e9243..1ef024bd08 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 `` 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 diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 95c12df5da..f7078d4704 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -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