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

* Refactor SSR test helpers * Add SSR tests for transition * Don’t overwrite classes during SSR when rendering fragments * Update changelog
2023-01-12 11:17:51 -05:00
parent 08c083768f
commit aac78d52b7
10 changed files with 205 additions and 113 deletions
@@ -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

@@ -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(<Example />)
+      let { contents } = await renderSSR(<Example />)

      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(<Example defaultIndex={1} />)
+      let { contents } = await renderSSR(<Example defaultIndex={1} />)

      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(<Example />)
+      const { contents } = await renderHydrate(<Example />)

      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(<Example defaultIndex={1} />)
+      const { contents } = await renderHydrate(<Example defaultIndex={1} />)

      expect(contents).not.toContain(`Content 1`)
      expect(contents).toContain(`Content 2`)
@@ -67,68 +64,3 @@ describe('Rendering', () => {
    })
  })
 })
-
-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())
-}
@@ -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')
+    })
+  })
+})
@@ -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())
+}
@@ -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<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(
@@ -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
        )
      )
    }
@@ -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

@@ -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())
-}
@@ -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')
+    })
+  })
+})
@@ -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,
@@ -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())
+}