test: edge cases for server action #745

himself65 · 2024-06-13T04:37:45Z

I was trying to make vercel-ai + waku, but found some bugs, let me see if I can reproduce here

vercel · 2024-06-13T04:37:48Z

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

Name	Status	Preview	Updated (UTC)
waku	❌ Failed (Inspect)		Jul 10, 2024 4:39am

codesandbox-ci · 2024-06-13T04:38:07Z

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.

e2e/fixtures/rsc-basic/modules/ai/src/server.js

himself65 · 2024-06-18T08:12:27Z

packages/waku/src/lib/renderers/rsc-renderer.ts

@@ -188,7 +198,46 @@ export async function renderRsc(
    ) {
      // XXX This doesn't support streaming unlike busboy
      const formData = parseFormData(bodyStr, contentType);
-      args = await decodeReply(formData);


There are something wrong, formData could include server references which need webpack runtime @dai-shi

https://github.com/facebook/react/blob/ddcecbbebf6af3fa32a323c0591dad1f63587aeb/packages/react-server/src/ReactFlightReplyServer.js#L927-L941

Alright. It's reasonable that it requires the second argument moduleMap.

himself65 · 2024-06-18T08:13:07Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+      const moduleMap = new Proxy({} as Record<string, ImportManifestEntry>, {
+        get(_target, rsfId: string): ImportManifestEntry {
+          const [fileId, name] = rsfId.split('#') as [string, string];
+          // fixme: race condition, server actions are not initialized in the first time


As said, I think we should initialize all server actions before running RSC otherwise it will crash for the first time

for next.js, they have a global server action map in the runtime

https://github.com/vercel/next.js/blob/86dbcbea431f2fd85d94a72a9c2d35781154a5d7/packages/next/src/server/app-render/encryption-utils.ts#L138-L158

I still need to understand the issue, but it feels like we want lazy evaluation.

himself65 · 2024-06-18T08:13:17Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+                  .then((m: any) => {
+                    moduleCache.set(
+                      fileId,
+                      Object.fromEntries(m['__waku_serverActions']),


Yeah... I hope we can improve it.

himself65 · 2024-06-18T08:13:40Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+          }
+          return {
+            id: fileId,
+            chunks: [],


chunks: [fileId] is not necessary after reading the source code fo next.js

Are you sure? I mean waku's approach might be different from nextjs's one.

himself65 · 2024-06-18T08:20:09Z

packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts

@@ -5,10 +5,25 @@ import { EXTENSIONS } from '../config.js';
 import { extname } from '../utils/path.js';
 import { parseOpts } from '../utils/swc.js';

-const collectExportNames = (mod: swc.Module) => {
+const collectExportNames = (


Here are other issues:

non export sever action could cause runtime error

nested server action could cause runtime error

const actionsMap = { foo: () => { "use server" // ... } }

Those are exist in vercel/ai/rsc source so we need to support them

Okay, I missed such use cases.
This pattern, inline "use server", is only allowed in the server files, right?

Hm, I made a test case #771, but it looks like it's transforming well.
Can you give me a failing case?

This is also tried:

const actionsMap = createActions({ foo: () => { "use server" // ... } })

himself65

This PR little mess up, but should separate into three parts:

Test some RSC edge cases
RSC transform plugin
RSC module map resolution

dai-shi

This looks like a good improvement.
If possible, we would like to split into multiple PRs and merge gradually.
(And, we definitely need unit tests for transformation. 😓 )

dai-shi · 2024-06-20T02:38:09Z

packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts

@@ -5,10 +5,25 @@ import { EXTENSIONS } from '../config.js';
 import { extname } from '../utils/path.js';
 import { parseOpts } from '../utils/swc.js';

-const collectExportNames = (mod: swc.Module) => {
+const collectExportNames = (


Okay, I missed such use cases.
This pattern, inline "use server", is only allowed in the server files, right?

dai-shi · 2024-06-20T02:39:47Z

packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts

      newCode += `
 export ${name === 'default' ? name : `const ${name} =`} registerClientReference(() => { throw new Error('It is not possible to invoke a client function from the server: ${getClientId(id)}#${name}'); }, '${getClientId(id)}', '${name}');
 `;
    }
    return newCode;
-  } else if (hasUseServer) {


I'm not sure if we can remove this branch. It's for another use case unless I'm mistaken.

dai-shi · 2024-06-20T02:40:56Z

packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts

      newCode += `
 if (typeof ${name} === 'function') {
  registerServerReference(${name}, '${getServerId(id)}', '${name}');
 }
 `;
+      if (internal.has(name)) {
+        newCode += `export { ${name} };\n`;


This doesn't feel correct. Possible name collisions.

dai-shi · 2024-06-20T02:41:54Z

packages/waku/src/lib/renderers/html-renderer.ts

-        }
-      >
-    >,
+    {} as Record<string, Record<string, ImportManifestEntry>>,


Nice, but maybe we should have a separate PR.

dai-shi · 2024-06-20T02:42:54Z

packages/waku/src/lib/renderers/rsc-renderer.ts

@@ -12,6 +12,16 @@ import { parseFormData } from '../utils/form.js';
 import { streamToString } from '../utils/stream.js';
 import { decodeActionId } from '../renderers/utils.js';

+// HACK for react-server-dom-webpack without webpack


It seems like we should finish up #707 before this PR.

dai-shi · 2024-06-20T02:45:21Z

packages/waku/src/lib/renderers/rsc-renderer.ts

@@ -188,7 +198,46 @@ export async function renderRsc(
    ) {
      // XXX This doesn't support streaming unlike busboy
      const formData = parseFormData(bodyStr, contentType);
-      args = await decodeReply(formData);


Alright. It's reasonable that it requires the second argument moduleMap.

dai-shi · 2024-06-20T02:47:56Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+      const moduleMap = new Proxy({} as Record<string, ImportManifestEntry>, {
+        get(_target, rsfId: string): ImportManifestEntry {
+          const [fileId, name] = rsfId.split('#') as [string, string];
+          // fixme: race condition, server actions are not initialized in the first time


I still need to understand the issue, but it feels like we want lazy evaluation.

dai-shi · 2024-06-20T02:48:29Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+                  .then((m: any) => {
+                    moduleCache.set(
+                      fileId,
+                      Object.fromEntries(m['__waku_serverActions']),


Yeah... I hope we can improve it.

dai-shi · 2024-06-20T02:49:35Z

packages/waku/src/lib/renderers/rsc-renderer.ts

+          }
+          return {
+            id: fileId,
+            chunks: [],


Are you sure? I mean waku's approach might be different from nextjs's one.

himself65 · 2024-06-21T20:39:56Z

Are you sure? I mean waku's approach might be different from nextjs's one.

It depends on what's the real behavior from webpack, I need double check that

dai-shi · 2024-07-02T13:24:39Z

I reverted the src code, but why do all e2e tests fail... should only a new one fail??

Fix some RSC case, will fix #745. In the future, I will migrate the logic to https://github.com/himself65/react-server and keep same output Some issues still happening: 1. `export default async function () {"use server"}` cannot be RSCA 2. `export function a() { "use server" }` should throw error, because it's not async --------- Co-authored-by: daishi <daishi@axlight.com>

…f65/20240612/actions # Conflicts: # e2e/fixtures/rsc-basic/package.json # pnpm-lock.yaml

Upstream: pnpm/action-setup#135 (comment)

Fix some RSC case, will fix dai-shi#745. In the future, I will migrate the logic to https://github.com/himself65/react-server and keep same output Some issues still happening: 1. `export default async function () {"use server"}` cannot be RSCA 2. `export function a() { "use server" }` should throw error, because it's not async --------- Co-authored-by: daishi <daishi@axlight.com>

dai-shi#774 (comment)

…hi#779) Hmm, this is going to be a big problem with ESM/CJS compat... Maybe related: dai-shi#421, dai-shi#428

- downgrade pnpm

I was hoping to use `-rc.0` for a while, but it has a bug and `-rc.<num>` has not been maintained. Let's use nightly rc version. close dai-shi#766

himself65 · 2024-07-10T03:53:50Z

working on decodeReply

… himself65/20240612/actions # Conflicts: # e2e/fixtures/rsc-basic/package.json # pnpm-lock.yaml

himself65 · 2024-07-10T05:34:42Z

PR is to complex to rebease/merge, create a new one #785

vercel bot deployed to Preview June 13, 2024 04:38 View deployment

himself65 force-pushed the himself65/20240612/actions branch from 90a553c to 40aff57 Compare June 13, 2024 04:39

himself65 marked this pull request as ready for review June 13, 2024 04:42

himself65 changed the title ~~test: cover more cases for server actions~~ [WIP] test: cover more cases for server actions Jun 14, 2024

himself65 commented Jun 17, 2024

View reviewed changes

e2e/fixtures/rsc-basic/modules/ai/src/server.js Outdated Show resolved Hide resolved

himself65 mentioned this pull request Jun 17, 2024

wrap sever action will cause runtime error #749

Closed

himself65 changed the title ~~[WIP] test: cover more cases for server actions~~ fix: cover more cases for server actions Jun 18, 2024

vercel bot deployed to Preview June 18, 2024 04:04 View deployment

himself65 force-pushed the himself65/20240612/actions branch from 8fe2383 to e1b49ca Compare June 18, 2024 04:07

fix: edge cases for server action

355e558

himself65 force-pushed the himself65/20240612/actions branch from e1b49ca to 355e558 Compare June 18, 2024 04:08

himself65 changed the title ~~fix: cover more cases for server actions~~ fix: edge cases for server action Jun 18, 2024

himself65 added 3 commits June 17, 2024 21:09

fix: remove unused console.log

c5934c8

fix: typo

0de7663

fix: input could be another server action

5c6b67c

vercel bot deployed to Preview June 18, 2024 07:57 View deployment

fix: update code

6fef60b

vercel bot had a problem deploying to Preview June 18, 2024 08:11 Failure

himself65 commented Jun 18, 2024

View reviewed changes

dai-shi reviewed Jun 20, 2024

View reviewed changes

dai-shi mentioned this pull request Jun 28, 2024

v1 Roadmap #24

Open

Merge branch 'main' into himself65/20240612/actions

65c6ecc

vercel bot deployed to Preview July 2, 2024 13:12 View deployment

avoid disabling ts ban comment

00efab2

himself65 mentioned this pull request Jul 3, 2024

fix: server action cases #774

Merged

dai-shi closed this in #774 Jul 4, 2024

dai-shi reopened this Jul 4, 2024

Merge branch 'main' into himself65/20240612/actions

1339070

vercel bot deployed to Preview July 4, 2024 04:04 View deployment

himself65 and others added 17 commits July 5, 2024 00:36

Merge remote-tracking branch 'refs/remotes/upstream/main' into himsel…

c64150f

…f65/20240612/actions # Conflicts: # e2e/fixtures/rsc-basic/package.json # pnpm-lock.yaml

fix: lock

237e07b

fix: edge cases for server action

a37303a

fix: remove unused console.log

4f8aef3

fix: typo

409cef7

fix: input could be another server action

366aadc

fix: update code

daeb9f2

avoid disabling ts ban comment

256cc41

ci: update pnpm setup (dai-shi#775)

b7b88a1

Upstream: pnpm/action-setup#135 (comment)

chore(examples): make all server functions async (dai-shi#778)

a1d06b7

dai-shi#774 (comment)

fix(website): hack next-mdx-remote with vite config (DEV only) (dai-s…

f79441c

…hi#779) Hmm, this is going to be a big problem with ESM/CJS compat... Maybe related: dai-shi#421, dai-shi#428

chore: update dependencies (dai-shi#780)

3267bdf

- downgrade pnpm

update react rc (dai-shi#781)

ddb4980

I was hoping to use `-rc.0` for a while, but it has a bug and `-rc.<num>` has not been maintained. Let's use nightly rc version. close dai-shi#766

v0.21.0-beta.1

0a7ff13

fix: lock

67bad7c

Merge branch 'main' into himself65/20240612/actions

a8e579c

vercel bot deployed to Preview July 10, 2024 03:36 View deployment

Merge remote-tracking branch 'origin/himself65/20240612/actions' into…

75e7d18

… himself65/20240612/actions # Conflicts: # e2e/fixtures/rsc-basic/package.json # pnpm-lock.yaml

vercel bot had a problem deploying to Preview July 10, 2024 04:39 Failure

himself65 closed this Jul 10, 2024

himself65 deleted the himself65/20240612/actions branch July 10, 2024 07:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: edge cases for server action #745

test: edge cases for server action #745

himself65 commented Jun 13, 2024

vercel bot commented Jun 13, 2024 •

edited

Loading

codesandbox-ci bot commented Jun 13, 2024 •

edited

Loading

himself65 Jun 18, 2024

himself65 Jun 18, 2024

dai-shi Jun 20, 2024

himself65 Jun 18, 2024

himself65 Jun 18, 2024

dai-shi Jun 20, 2024

himself65 Jun 18, 2024

dai-shi Jun 20, 2024

himself65 Jun 18, 2024

dai-shi Jun 20, 2024

himself65 Jun 18, 2024

dai-shi Jun 20, 2024

dai-shi Jul 1, 2024

himself65 left a comment

dai-shi left a comment

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jul 1, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

dai-shi Jun 20, 2024

himself65 commented Jun 21, 2024

dai-shi commented Jul 2, 2024

himself65 commented Jul 10, 2024

himself65 commented Jul 10, 2024

test: edge cases for server action #745

test: edge cases for server action #745

Conversation

himself65 commented Jun 13, 2024

vercel bot commented Jun 13, 2024 • edited Loading

codesandbox-ci bot commented Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himself65 left a comment

Choose a reason for hiding this comment

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himself65 commented Jun 21, 2024

dai-shi commented Jul 2, 2024

himself65 commented Jul 10, 2024

himself65 commented Jul 10, 2024

vercel bot commented Jun 13, 2024 •

edited

Loading

codesandbox-ci bot commented Jun 13, 2024 •

edited

Loading