RSC: Support client-only package and include @tobbe.dev/rsc-test in s…

…moke test (#9367)

__fixtures__/test-project-rsc-external-packages/package.json

@@ -20,13 +20,5 @@
   "prisma": {
     "seed": "yarn rw exec seed"
   },
-  "packageManager": "[email protected]",
-  "resolutions": {
-    "[email protected]": "patch:vite@npm%3A4.4.9#./.yarn/patches/vite-npm-4.4.9-e845c1bbf8.patch"
-  },
-  "dependencies": {
-    "@tobbe.dev/rsc-test": "^0.0.1",
-    "client-only": "^0.0.1",
-    "server-only": "^0.0.1"
-  }
+  "packageManager": "[email protected]"
 }

__fixtures__/test-project-rsc-external-packages/web/package.json

@@ -14,8 +14,11 @@
     "@redwoodjs/forms": "7.0.0-canary.413",
     "@redwoodjs/router": "7.0.0-canary.413",
     "@redwoodjs/web": "7.0.0-canary.413",
+    "@tobbe.dev/rsc-test": "0.0.3",
+    "client-only": "0.0.1",
     "react": "0.0.0-experimental-e5205658f-20230913",
-    "react-dom": "0.0.0-experimental-e5205658f-20230913"
+    "react-dom": "0.0.0-experimental-e5205658f-20230913",
+    "server-only": "0.0.1"
   },
   "devDependencies": {
     "@redwoodjs/vite": "7.0.0-canary.413",

__fixtures__/test-project-rsc-external-packages/web/src/App.tsx

@@ -1,14 +1,12 @@
-// import { RscForm } from '@tobbe.dev/rsc-test'
+import { RscForm } from '@tobbe.dev/rsc-test'
 
 import { Assets } from '@redwoodjs/vite/assets'
 import { ProdRwRscServerGlobal } from '@redwoodjs/vite/rwRscGlobal'
 
 // @ts-expect-error no types
 import styles from './App.module.css'
-import { onSend } from './chat'
+import { onSend } from './actions'
 import { Counter } from './Counter'
-// TODO (RSC): Switch to RscForm from @tobbe.dev as commented above
-import { Form as RscForm } from './Form'
 
 import './App.css'
 

__fixtures__/test-project-rsc-external-packages/web/src/Counter.tsx

@@ -2,8 +2,7 @@
 
 import React from 'react'
 
-// TODO (RSC): Enable this
-// import 'client-only'
+import 'client-only'
 
 // @ts-expect-error no types
 import styles from './Counter.module.css'

__fixtures__/test-project-rsc-external-packages/web/src/actions.ts

@@ -0,0 +1,16 @@
+'use server'
+
+import { randomWords } from './words'
+
+export async function onSend(formData: FormData) {
+  console.log('formData entries:')
+  formData.forEach((value, key) => console.log(key, value))
+
+  await new Promise((resolve) => setTimeout(resolve, 50 + Math.random() * 50))
+
+  // Want to keep something like this in the codebase to test
+  // importing files that use the `server-only` package
+  const words = await randomWords(5)
+
+  return formData.get('message') + ': ' + words.join(' ')
+}

packages/vite/src/buildRscFeServer.ts

@@ -54,6 +54,15 @@ export const buildRscFeServer = async ({
           ...clientEntryFiles,
         },
         preserveEntrySignatures: 'exports-only',
+        output: {
+          // This is not ideal. See
+          // https://rollupjs.org/faqs/#why-do-additional-imports-turn-up-in-my-entry-chunks-when-code-splitting
+          // But we need it to prevent `import 'client-only'` from being
+          // hoisted into App.tsx
+          // TODO (RSC): Fix when https://github.com/rollup/rollup/issues/5235
+          // is resolved
+          hoistTransitiveImports: false,
+        },
       },
       manifest: 'client-build-manifest.json',
     },

packages/vite/src/rsc/rscRequestHandler.ts

@@ -48,6 +48,9 @@ export function createRscRequestHandler() {
       }
 
       if (rsfId) {
+        // TODO (RSC): For React Server Actions we need to limit the request
+        // size somehow
+        // https://nextjs.org/docs/app/api-reference/functions/server-actions#size-limitation
         if (req.headers['content-type']?.startsWith('multipart/form-data')) {
           const bb = busboy({ headers: req.headers })
           const reply = decodeReplyFromBusboy(bb)

packages/vite/src/rsc/rscWorker.ts

@@ -255,6 +255,9 @@ const resolveClientEntry = (
 ) => {
   const clientEntry = absoluteClientEntries[filePath]
 
+  console.log('absoluteClientEntries', absoluteClientEntries)
+  console.log('filePath', filePath)
+
   if (!clientEntry) {
     if (absoluteClientEntries['*'] === '*') {
       return config.base + path.relative(config.root, filePath)

packages/vite/src/waku-lib/build-server.ts

@@ -31,28 +31,38 @@ export async function serverBuild(
     root: rwPaths.web.base,
     ssr: {
       // Externalize everything except packages with files that have
-      // 'use client' in them
+      // 'use client' in them (which are the files in `clientEntryFiles`)
       // Files included in `noExternal` are files we want Vite to analyze
       // The values in the array here are compared to npm package names, like
       // 'react', 'core-js', @anthropic-ai/sdk', @redwoodjs/vite', etc
       // The map function below will return '..' for local files. That's not
       // very pretty, but it works. It just won't match anything.
-      noExternal: Object.values(clientEntryFiles).map((fname) => {
+      noExternal: Object.values(clientEntryFiles).map((fullPath) => {
+        // On Windows `fullPath` will be something like
+        // D:/a/redwood/test-project-rsc-external-packages/node_modules/@tobbe.dev/rsc-test/dist/rsc-test.es.js
         const relativePath = path.relative(
           path.join(rwPaths.base, 'node_modules'),
-          fname
+          fullPath
         )
-        const splitPath = relativePath.split('/')
+        // On Windows `relativePath` will be something like
+        // @tobbe.dev\rsc-test\dist\rsc-test.es.js
+        // So `splitPath` will in this case become
+        // ['@tobbe.dev', 'rsc-test', 'dist', 'rsc-test.es.js']
+        const splitPath = relativePath.split(path.sep)
 
-        // TODO (RSC): Verify this is correct. Need to find a scoped package
-        // that uses 'use client'
-        // Handle scoped packages
-        if (relativePath.startsWith('@')) {
-          return splitPath[0] + '/' + splitPath[1]
+        // Packages without scope. Full package name looks like: package_name
+        let packageName = splitPath[0]
+
+        // Handle scoped packages. Full package name looks like:
+        // @org_name/package_name
+        if (splitPath[0].startsWith('@')) {
+          // join @org_name with package_name
+          packageName = path.join(splitPath[0], splitPath[1])
         }
 
-        // Packages without scope
-        return splitPath[0]
+        console.log('noExternal packageName', packageName)
+
+        return packageName
       }),
       resolve: {
         externalConditions: ['react-server'],
@@ -97,6 +107,13 @@ export async function serverBuild(
             }
             return 'assets/[name].js'
           },
+          // This is not ideal. See
+          // https://rollupjs.org/faqs/#why-do-additional-imports-turn-up-in-my-entry-chunks-when-code-splitting
+          // But we need it to prevent `import 'client-only'` from being
+          // hoisted into App.tsx
+          // TODO (RSC): Fix when https://github.com/rollup/rollup/issues/5235
+          // is resolved
+          hoistTransitiveImports: false,
         },
       },
     },

tasks/smoke-tests/rsc-external-packages/tests/rsc-external-packages.spec.ts

@@ -9,7 +9,7 @@ test('Client components should work', async ({ page }) => {
 
   await page.locator('button').filter({ hasText: 'Increment' }).click()
 
-  const count = await page.locator('p').first().innerText()
+  const count = await page.locator('p').nth(2).innerText()
   expect(count).toMatch('Count: 1')
 
   page.close()
@@ -22,23 +22,21 @@ test('Submitting the form should return a response', async ({ page }) => {
   expect(h3).toMatch(/Hello Redwood RSAs!!/)
 
   const pageText = await page.locator('#redwood-app > div').innerText()
-  expect(pageText).toMatch('The form has been submitted 0 times.')
+  expect(pageText).toMatch('This form has been sent 0 times')
 
   await page.getByRole('textbox').fill('Hello World')
   await page.getByRole('button').getByText('Send').click()
 
   const submittedPageText = page.locator('#redwood-app > div')
-  await expect(submittedPageText).toHaveText(
-    /The form has been submitted 1 times./
-  )
+  await expect(submittedPageText).toHaveText(/This form has been sent 1 times/)
 
   // Expect an echo of our message back from the server
-  const echo = await page.locator('p').first().innerText()
+  const echo = await page.locator('p').nth(1).innerText()
   expect(echo).toMatch('Hello World')
 
   // Expect to get five (random) words back from the server
   const words = await page.locator('p').nth(1).innerText()
-  expect(words.split(' ')).toHaveLength(5)
+  expect(words.split('Hello World: ')[1].split(' ')).toHaveLength(5)
 
   page.close()
 })