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

Support for React Server Components #1209

Open
remy90 opened this issue May 9, 2023 · 111 comments
Open

Support for React Server Components #1209

remy90 opened this issue May 9, 2023 · 111 comments
Assignees
Labels

Comments

@remy90
Copy link

remy90 commented May 9, 2023

Describe the feature you'd like:

As a user of react 18 with NextJS (with app directory), I would like to render async server components

example:
// Page.tsx

const Page = async ({ params, searchParams }: PageProps) => {
    const asyncData = await fetchSomeDataAsynchronously()
    return (<foo {...asyncData} />
}

...
// Page.test.tsx

it('should render', () => {
    render(<Page />)
    expect(screen....).ToBe(....)
})

Extracting server page logic would be an alternative, but I think that would also significantly reduce the purpose of RTL if that were to become an encouraged architectural default.

Current progress, workarounds, and demo

@prakashtiwaari
Copy link

You can use like this:

it('should render', async() => {
    render(<Page />);
    await waitFor(()=> {
      expect(screen....).ToBe(....)
    });
});

@remy90
Copy link
Author

remy90 commented May 11, 2023

@prakashtiwaari Not quite.

It's render that's giving the issue. To provide the full error message:

'Page' cannot be used as a JSX component.
  Its return type 'Promise<Element>' is not a valid JSX element.
    
Type 'Promise<Element>' is missing the following properties from type 'ReactElement<any, any>': type, props, keyts(2786)

@remy90
Copy link
Author

remy90 commented May 11, 2023

A friend came up with the following:

const props = {
  params: { surveyType: '123' },
  searchParams: { journeyId: '456' },
}

const Result = await Page(props)
render(Result)

I'll wait for a moderator to close this but it would be nice to have async components supported inside render

@eps1lon

This comment was marked as resolved.

@nickmccurdy
Copy link
Member

A friend came up with the following:

const props = {
  params: { surveyType: '123' },
  searchParams: { journeyId: '456' },
}

const Result = await Page(props)
render(Result)

I'll wait for a moderator to close this but it would be nice to have async components supported inside render

I like this idea personally. It should be easy enough to introspect if a component is async by determining if it returns a Promise. Then we can await the Promise internally and either return a Promise in render or poll to return synchronously.

@Gpx
Copy link
Member

Gpx commented May 14, 2023

For those that are trying to mock API responses, this seems to be working for now:

test("should do stuff", async () => {
  global.fetch = jest.fn().mockResolvedValue({
    json: jest.fn().mockResolvedValue({ login: "Gio" }),
  });
  render(await Page());
  expect(screen.getByText("Hi Gio!")).toBeInTheDocument();
});

If you don't define global.fetch, you get ReferenceError: fetch is not defined.

I would love to be able to use MSW, but that's still not working with Next's model. See: https://twitter.com/ApiMocking/status/1656249306628915208?s=20

On a side note, even if we make this work, there's the issue of Next layout composition. If I render a Page component in my test, I most likely want to render also its layout(s). I understand this is not necessarily a concern that RTL should have, but we should keep it in mind. Next could provide an API for testing that builds the whole rendering tree.

@nickmccurdy
Copy link
Member

nickmccurdy commented May 15, 2023

We could document using layouts similarly to how we already document using providers: by importing them and passing them as the wrapper option of render.

import {render} from '@testing-library/react'
import Layout from './layout'
import Page from './page'

render(<Page />, {wrapper: Layout})

@Gpx has a good point that rendering nested layouts would be more challenging. We could try to expose an abstraction, but the paths to import from would be dynamic and depend on Next patterns. If this isn't practical, we can open an issue with Next upstream and ask for an API to consume internally.

Also if we change render to be compatible with returning promises, we should probably ship a breaking change so end users are aware they may need to change their tests (especially if they're using TypeScript).

@Gpx
Copy link
Member

Gpx commented May 15, 2023

I agree we should ask Next to provide an API. I'm thinking something like this:

const RSC = routeComponent('/some/path') // This will return an RSC with all nested layouts passing the correct props
render(RSC)

// With additional props
const RSC = routeComponent('/some/path', { foo: 1 })
render(RSC)

WDYT?


As for the breaking change, won't this be a feature? I don't think anyone was relying on render breaking for async components.

@nickmccurdy
Copy link
Member

nickmccurdy commented May 15, 2023

I'm doing more research into RSCs using Next, and I noticed some mistaken assumptions I had:

  1. Determining if a component uses suspense is not possible due to the halting problem as it would be valid for the component to sometimes not return a Promise, and we can't distinguish between a component not returning a Promise currently vs. ever.
  2. The new rendering async APIs don't return Promises, they return streams. So, we don't necessarily need to make our render function async, though we may need to poll to unwrap streams synchronously.

Overall, I think supporting React server components involves figuring out the following concerns:

  1. [react] Allow returning ReactNode from function components DefinitelyTyped/DefinitelyTyped#65135
  2. Figuring out how we can internally render async components (may involve using a different render API, and would require using experimental/canary React right now)
  3. Supporting bundling and routing strategies from different frameworks (we could use the adapter pattern to compose APIs from RSC frameworks such as Expose method to render whole route tree in testing vercel/next.js#50479)

@nickmccurdy
Copy link
Member

I agree we should ask Next to provide an API. I'm thinking something like this:

const RSC = routeComponent('/some/path') // This will return an RSC with all nested layouts passing the correct props
render(RSC)

// With additional props
const RSC = routeComponent('/some/path', { foo: 1 })
render(RSC)

WDYT?

@Gpx Shouldn't that be render(<RSC />)? Also are you suggesting the URL path or filesystem path here?

@tom-sherman
Copy link

tom-sherman commented May 15, 2023

The idea to render the server component as async client components seems to be the best idea I've seen so far.

I don't think it's suitable for RTL to bake Next.js' (or any other framework's) semantics into it's API. Async client components solve this by being agnostic of any framework.

I think this would retain the current RTL API with maybe the addition of a suspense wrapper - rendering nothing initially could be confusing so enforcing a fallback makes sense.

render(<Page />) // throws an error because there's no boundary (I think this would be default react behaviour)
render(<Suspense><Page /></Suspense>) // This would be ok

@nickmccurdy

This comment was marked as resolved.

@tom-sherman

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@tom-sherman

This comment was marked as resolved.

@tom-sherman
Copy link

tom-sherman commented May 16, 2023

I do think it's easier to just mount the server component as a client component with createRoot though - this works in react-dom canary today.

See commit on your test repo here: tom-sherman/rsc-testing@9e4aa67

I'm not sure how RTL can support rendering of the root layout in Next.js, but as I said before I don't think it should - at least not part of it's core API. The root layout returning <html> is a Next.js-specific idiom, not all server component frameworks choose to work this way.

@nickmccurdy
Copy link
Member

nickmccurdy commented May 16, 2023

Thanks for the branch, that's interesting. I'd still like to understand why this works and onAllReady doesn't though, as that's supposed to be used for SSG where having suspense fallbacks everywhere wouldn't be accessible. Also, while I understand why your version of the test needs to await for a suspended render, I'd rather not make end users worry about understanding this if there's a way to ensure certain components load automatically. Additionally, both branches still have act warnings, though I have no idea how to fix this as I've already tried using it in various ways.

@tom-sherman
Copy link

Also, while I understand why your version of the test needs to await for a suspended render, I'd rather not make end users worry about understanding this if there's a way to ensure certain components load automatically

You're going to need an await somewhere - you can't syncronously block because it's an async component. I suppose you could have a builtin suspense boundary and have the user await render(<Page />) but as soon as the user adds a suspense boundary somewhere they're gonna need to use waitFor anyway.

@nickmccurdy
Copy link
Member

That's closer to what I was thinking originally. Though, I think I'm confused about how this is working, but I'll reply here again when I resolve it.

@Gpx
Copy link
Member

Gpx commented May 17, 2023

I agree we should ask Next to provide an API. I'm thinking something like this:

const RSC = routeComponent('/some/path') // This will return an RSC with all nested layouts passing the correct props
render(RSC)

// With additional props
const RSC = routeComponent('/some/path', { foo: 1 })
render(RSC)

WDYT?

@Gpx Shouldn't that be render(<RSC />)? Also are you suggesting the URL path or filesystem path here?

No, I think it should be render(RSC) where RSC is something like <Component {...nextProps} >. Alternatively routeComponent could return a component and its props:

const { Component, props } = routeComponent('/some/path')
render(<Component {...props} />)

We need not only the components tree but also the props that Next is passing.


The URL should be passed to the method, not the filesystem path. If we want to simulate a real user interacting with the app they'll use URLs.


To be clear, I'm not saying we should implement this in RTL, but rather that we should ask the Next team to provide it since it will be helpful for anyone doing tests.

@nickmccurdy
Copy link
Member

nickmccurdy commented May 17, 2023

I'm not sure we need it to return params, since you can just choose what params to render in your test by changing the URL.

@Gpx
Copy link
Member

Gpx commented May 18, 2023

I'm not sure we need it to return params, since you can just choose what params to render in your test by changing the URL.

Say you want to test the URL /foo/bar/baz. What are the params? Well, it depends on your file system:

Route params
app/foo/[slug]/baz { slug: 'bar' }
app/foo/bar/[slug] { slug: 'baz' }
app/foo/[[...slug]] { slug: ['bar', 'baz'] }

I can create the params object in my tests and pass it to the component, but if later I modify the filesystem, I might break my code, and my tests will still work.

If we want to keep the render(<Component />) format rather than render(Component) Component could just render the tree passing the correct params.

@tom-sherman
Copy link

Do we agree that RTL (at least in the core API) shouldn't support this kind of routing stuff? If so probably best to split that conversation out into a different issue?

@Gpx

This comment was marked as resolved.

@nickmccurdy
Copy link
Member

nickmccurdy commented May 18, 2023

If we want to keep the render(<Component />) format rather than render(Component) Component could just render the tree passing the correct params.

@Gpx Yes, I think that's simpler, and I'd rather avoid exposing the implementation detail of param values.

Do we agree that RTL (at least in the core API) shouldn't support this kind of routing stuff? If so probably best to split that conversation out into a different issue?

@tom-sherman I'm not suggesting we add a Next specific app router API directly into Testing Library. However, I'd like us to have either docs or some sort of adapter/facade/etc. design pattern that composes a Next routing primitive.

@DonikaV

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@DonikaV

This comment was marked as resolved.

leerob pushed a commit to vercel/next.js that referenced this issue Dec 13, 2023
This PR updates the testing guides to use App Router and TypeScript,
also updates `/examples` to show `app` and `pages` examples.

## Overview

- [x] Create a new "Testing" section that is shared between `app` and
`pages`.
- [x] Explain the differences between E2E, unit testing, component
testing, etc.
- [x] Recommend E2E for `async` components as currently none of the
tools support it.
- [x] Update setup guides for **Cypress**, **Playwright**, and **Jest**
with latest config options, and examples for `app` and `pages`.
- [x] Add new guide for **Vitest**
- [x] Clean up `/examples`: use TS, show `app` and `pages` examples,
match docs config

## Cypress

- [x] E2E Tests
- [x] Component Testing
  - [x] Client Components
  - [x] Server Components
  - [ ] `async` components

**Blockers:** 
- TS: `Option 'bundler' can only be used when 'module' is set to
'es2015' or later`. In **tsconfig.json** compilerOptions, Next.js uses
"moduleResolution": "bundler", changing it to "node" fixes the issue but
it can have repercussions.
  - cypress-io/cypress#27731 
- Version 14 is currently not supported for component testing
  - cypress-io/cypress#28185

## Playwright

- [x] E2E Tests

## Jest

- [x] Unit Testing
   - [x] Client Components
   - [x] Server Components
- [ ] `async` components:
testing-library/react-testing-library#1209
   - [x]  'server-only': #54891
- [x] Snapshot Testing

**Blockers:**
- TS: testing-library/jest-dom#546
- None of the solutions in the issue work with Next.js v14.0.4 and TS v5

## Vitest 

- [x] Unit Testing
  - [x] Client Components
  - [x] Server Components
  - [ ] `async` components
  - [x] 'server-only'
 - [x] Update vitest example
- [x] Handles CSS, and CSS modules imports
- [x] Handles next/image

## Other

- #47448
- #47299
@robinvandenb

This comment was marked as resolved.

@aurorascharff

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@aurorascharff

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

agustints pushed a commit to agustints/next.js that referenced this issue Jan 6, 2024
This PR updates the testing guides to use App Router and TypeScript,
also updates `/examples` to show `app` and `pages` examples.

## Overview

- [x] Create a new "Testing" section that is shared between `app` and
`pages`.
- [x] Explain the differences between E2E, unit testing, component
testing, etc.
- [x] Recommend E2E for `async` components as currently none of the
tools support it.
- [x] Update setup guides for **Cypress**, **Playwright**, and **Jest**
with latest config options, and examples for `app` and `pages`.
- [x] Add new guide for **Vitest**
- [x] Clean up `/examples`: use TS, show `app` and `pages` examples,
match docs config

## Cypress

- [x] E2E Tests
- [x] Component Testing
  - [x] Client Components
  - [x] Server Components
  - [ ] `async` components

**Blockers:** 
- TS: `Option 'bundler' can only be used when 'module' is set to
'es2015' or later`. In **tsconfig.json** compilerOptions, Next.js uses
"moduleResolution": "bundler", changing it to "node" fixes the issue but
it can have repercussions.
  - cypress-io/cypress#27731 
- Version 14 is currently not supported for component testing
  - cypress-io/cypress#28185

## Playwright

- [x] E2E Tests

## Jest

- [x] Unit Testing
   - [x] Client Components
   - [x] Server Components
- [ ] `async` components:
testing-library/react-testing-library#1209
   - [x]  'server-only': vercel#54891
- [x] Snapshot Testing

**Blockers:**
- TS: testing-library/jest-dom#546
- None of the solutions in the issue work with Next.js v14.0.4 and TS v5

## Vitest 

- [x] Unit Testing
  - [x] Client Components
  - [x] Server Components
  - [ ] `async` components
  - [x] 'server-only'
 - [x] Update vitest example
- [x] Handles CSS, and CSS modules imports
- [x] Handles next/image

## Other

- vercel#47448
- vercel#47299
@nickmccurdy
Copy link
Member

nickmccurdy commented Jan 18, 2024

The React team has announced that server components will likely be released in version 19, not 18.3, so we may want to merge issues involving both server components and the breaking changes in 19.

As for my work, I'm still focused on updating our current progress, workarounds, and demo along with related resources like Are we RSC yet? and react-unpin. I don't have time to continue working on renderServer yet, but it's still a longer team goal of mine.

@Ruffeng

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@Ruffeng

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@Ruffeng

This comment was marked as resolved.

@nickmccurdy
Copy link
Member

Next app router pins React 18.3. To use the same version in your tests, run react-unpin, then add that exact version (without a range and with a lockfile) to package.json.

@Ruffeng

This comment was marked as resolved.

@farsabbutt
Copy link

farsabbutt commented Feb 21, 2024

@nickmccurdy @Gpx do you have any clue on how to tests an async RSC that has another async RSC nested inside? Seems that the workaround(no React canary with suspense) works for the top level async RSC, but if I try to add another component that uses the async keyword, then the error is being shown again: Error: Uncaught [Error: Objects are not valid as a React child (found: [object Promise]). If you meant to render a collection of children, use an array instead.]

The example structure would look something like

export async function MyRSCParent(){
 const data = await fetch(....)
 return <MyRSCChild />
}

async function MyRSCChild() {
 const otherData = await fetch(...)
 return <span>{otherData}</span<
}

I can move the fetch from the child to the parent and pass it as props, but would be very nice if every server component could have its own fetch without having to centralize it in the top level

I had the same scenario. I mocked the child RSC in the tests and it works:

// MyRSCParent.tsx
export async function MyRSCParent(){
 const data = await fetch(....)
 return <MyRSCChild />
}

// MyRSCChild.tsx
export async function MyRSCChild() {
 const otherData = await fetch(...)
 return <span>{otherData}</span<
}
// MyRSCParent.test.tsx
jest.mock('path/to/nested/child/component', () => {
  return {
  MyRSCChild: () => null
}
})

it ('renders the component', () => {
  render(await MyRSCParent())
})

This way you can just mock the RSC child component no matter how deeply nested it is in the tree. Perhaps it would be better if the mock returns the actual component after resolving its promise.

@nickmccurdy
Copy link
Member

Reminder: We recommend integration testing, rather than unit testing, without mocking components.

@ajeetchaulagain

This comment was marked as resolved.

@nickmccurdy

This comment was marked as resolved.

@tomitrescak
Copy link

Reminder: We recommend integration testing, rather than unit testing, without mocking components.

Buuut, it’s still ok to mock data retrieval, right, right?

@nickmccurdy
Copy link
Member

Personally I avoid it, but generally Testing Library recommends network level mocking with something like msw or Cypress.

@tomitrescak
Copy link

@nickmccurdy all seems well with React 19, but I am sstill getting stderror (which does not fail tests)

Warning: A component was suspended by an uncached promise. Creating promises inside a Client Component or hook is not yet supported, except via a Suspense-compatible library or framework.

I guess it can be safely ignored ?

@nickmccurdy
Copy link
Member

This appears to be technically but not officially supported. Personally I'm fine with ignoring it, but keep in mind this won't be as stable as an end to end test or the RTL server I've been planning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests