Enable progressive enhanced form actions through decodeAction (#49187)

This uses the new built-in progressive enhancement features of React. These always use `multipart/form-data` atm. When one comes in that's not a fetch, we can use `decodeAction` to get a resolved function. This also ensures that we can test this by passing disableJavaScript to tests. This disables JS for the context.
vercel · May 4, 2023 · b877de1 · b877de1
1 parent 83b774e
commit b877de1
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 41 deletions.
diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts
@@ -596,7 +596,7 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
 
     export * as serverHooks from 'next/dist/client/components/hooks-server-context'
 
-    export { renderToReadableStream, decodeReply } from 'react-server-dom-webpack/server.edge'
+    export { renderToReadableStream, decodeReply, decodeAction } from 'react-server-dom-webpack/server.edge'
     export const __next_app_webpack_require__ = __webpack_require__
     export { preloadStyle, preloadFont, preconnect } from 'next/dist/server/app-render/rsc/preloads'
     

diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts
@@ -171,7 +171,7 @@ export async function handleAction({
 }): Promise<undefined | RenderResult | 'not-found'> {
   let actionId = req.headers[ACTION.toLowerCase()] as string
   const contentType = req.headers['content-type']
-  const isFormAction =
+  const isURLEncodedAction =
     req.method === 'POST' && contentType === 'application/x-www-form-urlencoded'
   const isMultipartAction =
     req.method === 'POST' && contentType?.startsWith('multipart/form-data')
@@ -181,7 +181,7 @@ export async function handleAction({
     typeof actionId === 'string' &&
     req.method === 'POST'
 
-  if (isFetchAction || isFormAction || isMultipartAction) {
+  if (isFetchAction || isURLEncodedAction || isMultipartAction) {
     let bound = []
 
     const workerName = 'app' + pathname
@@ -210,7 +210,7 @@ export async function handleAction({
       await actionAsyncStorage.run({ isAction: true }, async () => {
         if (process.env.NEXT_RUNTIME === 'edge') {
           // Use react-server-dom-webpack/server.edge
-          const { decodeReply } = ComponentMod
+          const { decodeReply, decodeAction } = ComponentMod
 
           const webRequest = req as unknown as WebNextRequest
           if (!webRequest.body) {
@@ -220,7 +220,14 @@ export async function handleAction({
           if (isMultipartAction) {
             // TODO-APP: Add streaming support
             const formData = await webRequest.request.formData()
-            bound = await decodeReply(formData, serverModuleMap)
+            if (isFetchAction) {
+              bound = await decodeReply(formData, serverModuleMap)
+            } else {
+              const action = await decodeAction(formData, serverModuleMap)
+              await action()
+              // Skip the fetch path
+              return
+            }
           } else {
             let actionData = ''
 
@@ -234,16 +241,9 @@ export async function handleAction({
               actionData += new TextDecoder().decode(value)
             }
 
-            if (isFormAction) {
+            if (isURLEncodedAction) {
               const formData = formDataFromSearchQueryString(actionData)
-              actionId = formData.get('$$id') as string
-
-              if (!actionId) {
-                // Return if no action ID is found, it could be a regular POST request
-                return
-              }
-              formData.delete('$$id')
-              bound = [formData]
+              bound = await decodeReply(formData, serverModuleMap)
             } else {
               bound = await decodeReply(actionData, serverModuleMap)
             }
@@ -253,28 +253,41 @@ export async function handleAction({
           const {
             decodeReply,
             decodeReplyFromBusboy,
+            decodeAction,
           } = require(`react-server-dom-webpack/server.node`)
 
           if (isMultipartAction) {
-            const busboy = require('busboy')
-            const bb = busboy({ headers: req.headers })
-            req.pipe(bb)
+            if (isFetchAction) {
+              const busboy = require('busboy')
+              const bb = busboy({ headers: req.headers })
+              req.pipe(bb)
 
-            bound = await decodeReplyFromBusboy(bb, serverModuleMap)
+              bound = await decodeReplyFromBusboy(bb, serverModuleMap)
+            } else {
+              // React doesn't yet publish a busboy version of decodeAction
+              // so we polyfill the parsing of FormData.
+              const { Readable } = require('stream')
+              const UndiciRequest = require('next/dist/compiled/undici').Request
+              const fakeRequest = new UndiciRequest('http://localhost', {
+                method: 'POST',
+                headers: { 'Content-Type': req.headers['content-type'] },
+                body: Readable.toWeb(req),
+                duplex: 'half',
+              })
+              const formData = await fakeRequest.formData()
+              const action = await decodeAction(formData, serverModuleMap)
+              await action()
+              // Skip the fetch path
+              return
+            }
           } else {
             const { parseBody } =
               require('../api-utils/node') as typeof import('../api-utils/node')
             const actionData = (await parseBody(req, '1mb')) || ''
 
-            if (isFormAction) {
-              actionId = actionData.$$id as string
-              if (!actionId) {
-                // Return if no action ID is found, it could be a regular POST request
-                return
-              }
+            if (isURLEncodedAction) {
               const formData = formDataFromSearchQueryString(actionData)
-              formData.delete('$$id')
-              bound = [formData]
+              bound = await decodeReply(formData, serverModuleMap)
             } else {
               bound = await decodeReply(actionData, serverModuleMap)
             }
@@ -302,9 +315,7 @@ export async function handleAction({
         }
       })
 
-      if (actionResult) {
-        return actionResult
-      }
+      return actionResult
     } catch (err) {
       if (isRedirectError(err)) {
         if (process.env.NEXT_RUNTIME === 'edge') {

diff --git a/test/e2e/app-dir/actions/app-action.test.ts b/test/e2e/app-dir/actions/app-action.test.ts
@@ -80,7 +80,10 @@ createNextDescribe(
     })
 
     it('should support notFound', async () => {
-      const browser = await next.browser('/server')
+      const browser = await next.browser('/server', {
+        // TODO we should also test this with javascript on but not-found is not implemented yet.
+        disableJavaScript: true,
+      })
 
       await browser.elementByCss('#nowhere').click()
 

diff --git a/test/e2e/app-dir/actions/app/server/form.js b/test/e2e/app-dir/actions/app/server/form.js
@@ -15,6 +15,11 @@ async function nowhere() {
   notFound()
 }
 
+async function here() {
+  'use server'
+  // nothing
+}
+
 async function file(formData) {
   'use server'
   const file = formData.get('file')
@@ -25,19 +30,20 @@ export default function Form() {
   return (
     <>
       <hr />
-      <form method="POST" action="">
-        <input type="text" name="$$id" value={action.$$id} hidden readOnly />
+      <form action={action}>
         <input type="text" name="name" id="name" required />
         <button type="submit" id="submit">
           Submit
         </button>
       </form>
       <hr />
-      <form method="POST" action="">
-        <input type="text" name="$$id" value={nowhere.$$id} hidden readOnly />
-        <button type="submit" id="nowhere">
+      <form>
+        <button formAction={nowhere} type="submit" id="nowhere">
           Go nowhere
         </button>
+        <button formAction={here} type="submit" id="here">
+          Go here
+        </button>
       </form>
       <hr />
       <form action={file}>

diff --git a/test/lib/browsers/base.ts b/test/lib/browsers/base.ts
@@ -43,7 +43,11 @@ export class BrowserInterface implements PromiseLike<any> {
     })
   }
 
-  async setup(browserName: string, locale?: string): Promise<void> {}
+  async setup(
+    browserName: string,
+    locale: string,
+    javaScriptEnabled: boolean
+  ): Promise<void> {}
   async close(): Promise<void> {}
   async quit(): Promise<void> {}
 

diff --git a/test/lib/browsers/playwright.ts b/test/lib/browsers/playwright.ts
@@ -15,6 +15,7 @@ import path from 'path'
 let page: Page
 let browser: Browser
 let context: BrowserContext
+let contextHasJSEnabled: boolean = true
 let pageLogs: Array<{ source: string; message: string }> = []
 let websocketFrames: Array<{ payload: string | Buffer }> = []
 
@@ -47,8 +48,7 @@ export class Playwright extends BrowserInterface {
     this.eventCallbacks[event]?.delete(cb)
   }
 
-  async setup(browserName: string, locale?: string) {
-    if (browser) return
+  async setup(browserName: string, locale: string, javaScriptEnabled: boolean) {
     const headless = !!process.env.HEADLESS
     let device
 
@@ -62,8 +62,23 @@ export class Playwright extends BrowserInterface {
       }
     }
 
+    if (browser) {
+      if (contextHasJSEnabled !== javaScriptEnabled) {
+        // If we have switched from having JS enable/disabled we need to recreate the context.
+        await context?.close()
+        context = await browser.newContext({
+          locale,
+          javaScriptEnabled,
+          ...device,
+        })
+        contextHasJSEnabled = javaScriptEnabled
+      }
+      return
+    }
+
     browser = await this.launchBrowser(browserName, { headless })
-    context = await browser.newContext({ locale, ...device })
+    context = await browser.newContext({ locale, javaScriptEnabled, ...device })
+    contextHasJSEnabled = javaScriptEnabled
   }
 
   async launchBrowser(browserName: string, launchOptions: Record<string, any>) {

diff --git a/test/lib/browsers/selenium.ts b/test/lib/browsers/selenium.ts
@@ -46,7 +46,7 @@ export class Selenium extends BrowserInterface {
   private browserName: string
 
   // TODO: support setting locale
-  async setup(browserName: string, locale?: string) {
+  async setup(browserName: string, locale: string, javaScriptEnabled: boolean) {
     if (browser) return
     this.browserName = browserName
 

diff --git a/test/lib/next-webdriver.ts b/test/lib/next-webdriver.ts
@@ -61,6 +61,7 @@ export default async function webdriver(
     disableCache?: boolean
     beforePageLoad?: (page: any) => void
     locale?: string
+    disableJavaScript?: boolean
   }
 ): Promise<BrowserInterface> {
   let CurrentInterface: typeof BrowserInterface
@@ -77,6 +78,7 @@ export default async function webdriver(
     disableCache,
     beforePageLoad,
     locale,
+    disableJavaScript,
   } = options
 
   // we import only the needed interface
@@ -96,7 +98,7 @@ export default async function webdriver(
 
   const browser = new CurrentInterface()
   const browserName = process.env.BROWSER_NAME || 'chrome'
-  await browser.setup(browserName, locale)
+  await browser.setup(browserName, locale, !disableJavaScript)
   ;(global as any).browserName = browserName
 
   const fullUrl = getFullUrl(