test(ssr): Validate state synchronization between server and client in React 18 using Zustand (#903)

[NaamuKim]

Related Issues or Discussions

• [test] add spec with ssr #903

Summary

This change tests state synchronization between server and client using React 18 and Zustand. It verifies that the server-rendered markup reflects the initial state correctly, and this state is accurately updated on the client side. This test is executed only when the version of React is 18.

Please suggest me if you have any other tests that might be helpful!

Check List

• yarn run prettier for formatting code and docs

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
zustand-demo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 2, 2023 0:59am

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 24da89c:

Sandbox	Source
React	Configuration
React TypeScript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration
Next.js	Configuration
@pavlobu/zustand demo	Configuration

[dai-shi]

Can anyone review this PR please?

[dbritto-dev]

Some ideas to make it work on react versions under 18

[dbritto-dev]

We can skip any react version under 18

Suggested change

	import React, { useEffect } from 'react'
	import { act, screen } from '@testing-library/react'
	import { hydrateRoot } from 'react-dom/client'
	import { renderToString } from 'react-dom/server'
	import { describe, expect, it } from 'vitest'
	import { create } from 'zustand'
	const IS_REACT_18 = React.version.startsWith('18')
	import React, { useEffect } from 'react'
	if (!React.version.startsWith('18')) {
	describe.skip('skipping test on react version < 18', () => {})
	return
	}
	import { act, screen } from '@testing-library/react'
	import { hydrateRoot } from 'react-dom/client'
	import { renderToString } from 'react-dom/server'
	import { describe, expect, it } from 'vitest'
	import { create } from 'zustand'

[NaamuKim]

Oh, thank you for your review.
I understand your intention.
I resolved it by using the only method from vitest and dynamic import.
What do you think about it?

[dbritto-dev]

@NaamuKim that approach doesn't work you should completely avoid execute the next lines. Let's try with my suggestion.

[NaamuKim]

@dbritto-dev There's an issue with top-level returns not being allowed in ESM, causing errors.
I think your suggestion of using describe.skip is excellent, and I've configured it to conditionally branch describe based on the version.
Could you please confirm this?

[dbritto-dev]

We can omit this statement

Suggested change

	if (!IS_REACT_18) {
	it('Dummy test for React 17, ignore', () => {})
	return
	}

[dbritto-dev]

@dai-shi would you mind triggering the tests again?

[dbritto-dev]

Some minor tweaks

[dbritto-dev]

Move to the top and use vi.importActual instead of require

Suggested change

	// Using dynamic import for 'react-dom/client' as it’s not accessible in React versions below 18.
	// eslint-disable-next-line import/no-unresolved
	const { hydrateRoot } = await require('react-dom/client')

[dbritto-dev]

Suggested change

	interface BearStoreState {
	const { hydrateRoot } = await vi.importActual('react-dom/client')
	interface BearStoreState {

[NaamuKim]

I want to use it like this, but if I change it to "skipIf" and it runs, it throws an error because it doesn't actually skip the context.

describe.skipIf(!React.version.startsWith('18'))(
  'ssr behavior with react 18',
  async () => {
    const {
      hydrateRoot,
    }: {
      hydrateRoot: (
        container: Element | Document,
        initialChildren: React.ReactNode
      ) => void
    } = await vi.importActual('react-dom/client')

throw error with a message Error: Failed to load url react-dom/client (resolved id: react-dom/client). Does the file exist?

If I move the other context inside the it context, it resolves the issue.

I've applied the changes with it.
Please take a look and feel free to share any further suggestions or improvements

[dbritto-dev]

Yeah I mean to the top of the context

[dbritto-dev]

Add vi on imports

Suggested change

	import { describe, expect, it } from 'vitest'
	import { describe, expect, it, vi } from 'vitest'

[dbritto-dev]

Let's use describe.skipIf

Suggested change

	const describeTest = React.version.startsWith('18') ? describe : describe.skip
	describeTest('ssr behavior with react 18', () => {
	describe.skipIf(!React.version.startsWith('18'))('ssr behavior with react 18', () => {

[dbritto-dev]

Suggested change

	const {
	hydrateRoot,
	}: {
	hydrateRoot: (
	container: Element | Document,
	initialChildren: React.ReactNode
	) => void
	} = await vi.importActual('react-dom/client')
	const { hydrateRoot } = await vi.importActual<typeof import('react-dom/client')>('react-dom/client')

[NaamuKim]

Thank you for the suggestion. I've made the requested changes as you suggested.

[dai-shi]

Thanks for your contribution!

[dai-shi]

Oh, I didn't know about skipIf!

We should use it in jotai too:
https://github.com/pmndrs/jotai/blob/d6afb777705eb3a90385817fd6db30db101f6caa/tests/react/transition.test.tsx#L12C1-L16

Free free to open a PR there.

[NaamuKim]

I am also glad to have learned about skipIf, and I will apply refactoring accordingly in Jotai as well

[dbritto-dev]

I guess we are good to merge the changes :D. BTW, good job.

[NaamuKim]

Thank you so much for your review.
I'm delighted to learn about the useful functions of vitest. I'll continue to check if there are any opportunities for further contributions.