From da386d78cf982edd542a8bd02197f3cf46975888 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Wed, 31 Jul 2024 17:36:50 +0200 Subject: [PATCH] Add failing end-to-end test for an action that returns client components This playwright test is using the Flight Fixture to demonstrate the Flight Reply equivalent of the scenario that was fixed in #30528 for the Flight Client. It's basically an advanced case of what was outlined in #28564, returning a client component from a server action that is used in `useActionState`. In addition, the client component uses another element twice, which leads to the element's props being deduped. Resolving those references needs to be handled specifically, both in the Flight Client (done in #30528), as well as in the temporary references of the Flight Reply Client (and possibly Flight Reply Server?). The test should probably be converted into a unit test, e.g. in `packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js` or `packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js`. --- .../flight/__tests__/__e2e__/action.test.js | 35 +++++++++++++++++++ fixtures/flight/server/region.js | 23 ++++++++---- fixtures/flight/src/App.js | 4 ++- fixtures/flight/src/Deduped.js | 8 +++++ fixtures/flight/src/TemporaryReferences.js | 14 ++++++++ fixtures/flight/src/actions.js | 13 +++++++ fixtures/flight/src/index.js | 16 +++++++-- 7 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 fixtures/flight/__tests__/__e2e__/action.test.js create mode 100644 fixtures/flight/src/Deduped.js create mode 100644 fixtures/flight/src/TemporaryReferences.js diff --git a/fixtures/flight/__tests__/__e2e__/action.test.js b/fixtures/flight/__tests__/__e2e__/action.test.js new file mode 100644 index 0000000000000..e52623322ccd3 --- /dev/null +++ b/fixtures/flight/__tests__/__e2e__/action.test.js @@ -0,0 +1,35 @@ +// @ts-check + +import {test, expect} from '@playwright/test'; + +test('action returning client component with deduped references', async ({ + page, +}) => { + const pageErrors = []; + + page.on('pageerror', error => { + pageErrors.push(error.stack); + }); + + await page.goto('/'); + + const button = await page.getByRole('button', { + name: 'Return element from action', + }); + + await button.click(); + + await expect( + page.getByTestId('temporary-references-action-result') + ).toHaveText('Hello'); + + // Click the button one more time to send the previous result (i.e. the + // returned element) back to the server. + await button.click(); + + await expect(pageErrors).toEqual([]); + + await expect( + page.getByTestId('temporary-references-action-result') + ).toHaveText('HelloHello'); +}); diff --git a/fixtures/flight/server/region.js b/fixtures/flight/server/region.js index bc4ba05ddf3b4..cb1aac5da8d73 100644 --- a/fixtures/flight/server/region.js +++ b/fixtures/flight/server/region.js @@ -50,7 +50,7 @@ const {readFile} = require('fs').promises; const React = require('react'); -async function renderApp(res, returnValue, formState) { +async function renderApp(res, returnValue, formState, temporaryReferences) { const {renderToPipeableStream} = await import( 'react-server-dom-webpack/server' ); @@ -101,7 +101,9 @@ async function renderApp(res, returnValue, formState) { ); // For client-invoked server actions we refresh the tree and return a return value. const payload = {root, returnValue, formState}; - const {pipe} = renderToPipeableStream(payload, moduleMap); + const {pipe} = renderToPipeableStream(payload, moduleMap, { + temporaryReferences, + }); pipe(res); } @@ -110,8 +112,13 @@ app.get('/', async function (req, res) { }); app.post('/', bodyParser.text(), async function (req, res) { - const {decodeReply, decodeReplyFromBusboy, decodeAction, decodeFormState} = - await import('react-server-dom-webpack/server'); + const { + decodeReply, + decodeReplyFromBusboy, + decodeAction, + decodeFormState, + createTemporaryReferenceSet, + } = await import('react-server-dom-webpack/server'); const serverReference = req.get('rsc-action'); if (serverReference) { // This is the client-side case @@ -124,15 +131,17 @@ app.post('/', bodyParser.text(), async function (req, res) { throw new Error('Invalid action'); } + const temporaryReferences = createTemporaryReferenceSet(); + let args; if (req.is('multipart/form-data')) { // Use busboy to streamingly parse the reply from form-data. const bb = busboy({headers: req.headers}); - const reply = decodeReplyFromBusboy(bb); + const reply = decodeReplyFromBusboy(bb, {}, {temporaryReferences}); req.pipe(bb); args = await reply; } else { - args = await decodeReply(req.body); + args = await decodeReply(req.body, {}, {temporaryReferences}); } const result = action.apply(null, args); try { @@ -142,7 +151,7 @@ app.post('/', bodyParser.text(), async function (req, res) { // We handle the error on the client } // Refresh the client and return the value - renderApp(res, result, null); + renderApp(res, result, null, temporaryReferences); } else { // This is the progressive enhancement case const UndiciRequest = require('undici').Request; diff --git a/fixtures/flight/src/App.js b/fixtures/flight/src/App.js index 027056c515021..1c9575dcfb43e 100644 --- a/fixtures/flight/src/App.js +++ b/fixtures/flight/src/App.js @@ -12,10 +12,11 @@ import Button from './Button.js'; import Form from './Form.js'; import {Dynamic} from './Dynamic.js'; import {Client} from './Client.js'; +import {TemporaryReferences} from './TemporaryReferences.js'; import {Note} from './cjs/Note.js'; -import {like, greet, increment} from './actions.js'; +import {like, greet, increment, returnElement} from './actions.js'; import {getServerState} from './ServerState.js'; @@ -61,6 +62,7 @@ export default async function App() { + diff --git a/fixtures/flight/src/Deduped.js b/fixtures/flight/src/Deduped.js new file mode 100644 index 0000000000000..1c694cd0092d8 --- /dev/null +++ b/fixtures/flight/src/Deduped.js @@ -0,0 +1,8 @@ +'use client'; + +import * as React from 'react'; + +export default function Deduped({children, thing}) { + console.log({thing}); + return
{children}
; +} diff --git a/fixtures/flight/src/TemporaryReferences.js b/fixtures/flight/src/TemporaryReferences.js new file mode 100644 index 0000000000000..c21a514ad3617 --- /dev/null +++ b/fixtures/flight/src/TemporaryReferences.js @@ -0,0 +1,14 @@ +'use client'; + +import * as React from 'react'; + +export function TemporaryReferences({action}) { + const [result, formAction] = React.useActionState(action, null); + + return ( +
+ +
{result}
+
+ ); +} diff --git a/fixtures/flight/src/actions.js b/fixtures/flight/src/actions.js index aa19871a9dcbb..463bb7cd98332 100644 --- a/fixtures/flight/src/actions.js +++ b/fixtures/flight/src/actions.js @@ -1,6 +1,8 @@ 'use server'; +import * as React from 'react'; import {setServerState} from './ServerState.js'; +import Deduped from './Deduped.js'; export async function like() { setServerState('Liked!'); @@ -22,3 +24,14 @@ export async function greet(formData) { export async function increment(n) { return n + 1; } + +export async function returnElement(prevElement) { + const text =
Hello
; + + return ( + + {prevElement} + {text} + + ); +} diff --git a/fixtures/flight/src/index.js b/fixtures/flight/src/index.js index b4b538a3a9992..6864fe927c22d 100644 --- a/fixtures/flight/src/index.js +++ b/fixtures/flight/src/index.js @@ -1,12 +1,18 @@ import * as React from 'react'; import {use, Suspense, useState, startTransition} from 'react'; import ReactDOM from 'react-dom/client'; -import {createFromFetch, encodeReply} from 'react-server-dom-webpack/client'; +import { + createFromFetch, + createTemporaryReferenceSet, + encodeReply, +} from 'react-server-dom-webpack/client'; // TODO: This should be a dependency of the App but we haven't implemented CSS in Node yet. import './style.css'; let updateRoot; +const temporaryReferences = createTemporaryReferenceSet(); + async function callServer(id, args) { const response = fetch('/', { method: 'POST', @@ -14,9 +20,12 @@ async function callServer(id, args) { Accept: 'text/x-component', 'rsc-action': id, }, - body: await encodeReply(args), + body: await encodeReply(args, {temporaryReferences}), + }); + const {returnValue, root} = await createFromFetch(response, { + callServer, + temporaryReferences, }); - const {returnValue, root} = await createFromFetch(response, {callServer}); // Refresh the tree with the new RSC payload. startTransition(() => { updateRoot(root); @@ -39,6 +48,7 @@ async function hydrateApp() { }), { callServer, + temporaryReferences, findSourceMapURL(fileName) { return ( document.location.origin +