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

test: edge cases for server action #745

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
355e558
fix: edge cases for server action
himself65 Jun 13, 2024
c5934c8
fix: remove unused console.log
himself65 Jun 18, 2024
0de7663
fix: typo
himself65 Jun 18, 2024
5c6b67c
fix: input could be another server action
himself65 Jun 18, 2024
6fef60b
fix: update code
himself65 Jun 18, 2024
65c6ecc
Merge branch 'main' into himself65/20240612/actions
dai-shi Jul 2, 2024
00efab2
avoid disabling ts ban comment
dai-shi Jul 2, 2024
1339070
Merge branch 'main' into himself65/20240612/actions
dai-shi Jul 4, 2024
c64150f
Merge remote-tracking branch 'refs/remotes/upstream/main' into himsel…
himself65 Jul 5, 2024
237e07b
fix: lock
himself65 Jul 5, 2024
a37303a
fix: edge cases for server action
himself65 Jun 13, 2024
4f8aef3
fix: remove unused console.log
himself65 Jun 18, 2024
409cef7
fix: typo
himself65 Jun 18, 2024
366aadc
fix: input could be another server action
himself65 Jun 18, 2024
daeb9f2
fix: update code
himself65 Jun 18, 2024
256cc41
avoid disabling ts ban comment
dai-shi Jul 2, 2024
b7b88a1
ci: update pnpm setup (#775)
himself65 Jul 4, 2024
21fcbe8
fix: server action cases (#774)
himself65 Jul 4, 2024
a1d06b7
chore(examples): make all server functions async (#778)
dai-shi Jul 4, 2024
f79441c
fix(website): hack next-mdx-remote with vite config (DEV only) (#779)
dai-shi Jul 5, 2024
3267bdf
chore: update dependencies (#780)
dai-shi Jul 5, 2024
ddb4980
update react rc (#781)
dai-shi Jul 5, 2024
0a7ff13
v0.21.0-beta.1
dai-shi Jul 5, 2024
67bad7c
fix: lock
himself65 Jul 5, 2024
a8e579c
Merge branch 'main' into himself65/20240612/actions
himself65 Jul 10, 2024
75e7d18
Merge remote-tracking branch 'origin/himself65/20240612/actions' into…
himself65 Jul 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions e2e/fixtures/rsc-basic/modules/ai/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"name": "ai",
"type": "module",
"version": "1.0.0",
"description": "Vercel AI mockup",
"exports": {
"./rsc": {
"types": "./src/index.d.ts",
"react-server": "./src/server.js",
"import": "./src/client.js"
}
},
"devDependencies": {
"react-dom": "^18",
"react-server-dom-webpack": "18.3.0-canary-eb33bd747-20240312"
},
"peerDependencies": {
"react": "^18 || ^19"
},
"private": true
}
8 changes: 8 additions & 0 deletions e2e/fixtures/rsc-basic/modules/ai/src/client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use client';
import { useActions } from './shared.js';

export { useActions };

export function createAI() {
throw new Error('You should not call createAI in the client side');
}
7 changes: 7 additions & 0 deletions e2e/fixtures/rsc-basic/modules/ai/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type { ReactNode } from 'react';

declare function createAI(options: {
actions: Record<string, any>;
}): (props: { children: ReactNode }) => ReactNode;

declare function useActions(): Record<string, any>;
25 changes: 25 additions & 0 deletions e2e/fixtures/rsc-basic/modules/ai/src/server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use server';
import { InternalProvider } from './shared.js';
import { jsx } from 'react/jsx-runtime';

async function innerAction({ action }, ...args) {
'use server';
return await action(...args);
}

function wrapAction(action) {
return innerAction.bind(null, { action });
}

export function createAI({ actions }) {
const wrappedActions = {};
for (const name in actions) {
wrappedActions[name] = wrapAction(actions[name]);
}
return function AI(props) {
return jsx(InternalProvider, {
actions: wrappedActions,
children: props.children,
});
};
}
19 changes: 19 additions & 0 deletions e2e/fixtures/rsc-basic/modules/ai/src/shared.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use client';
import { createContext, useContext } from 'react';
import { jsx } from 'react/jsx-runtime';

const ActionContext = createContext(null);

export function useActions() {
return useContext(ActionContext);
}

export function InternalProvider(props) {
return jsx('div', {
'data-testid': 'ai-internal-provider',
children: jsx(ActionContext.Provider, {
value: props.actions,
children: props.children,
}),
});
}
1 change: 1 addition & 0 deletions e2e/fixtures/rsc-basic/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"start": "waku start"
},
"dependencies": {
"ai": "link:./modules/ai",
"react": "19.0.0-rc-3da26163a3-20240704",
"react-dom": "19.0.0-rc-3da26163a3-20240704",
"react-server-dom-webpack": "19.0.0-rc-3da26163a3-20240704",
Expand Down
5 changes: 5 additions & 0 deletions e2e/fixtures/rsc-basic/src/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import { ClientCounter } from './ClientCounter.js';
import { ServerPing } from './ServerPing/index.js';
import { ServerBox } from './Box.js';
import { ServerProvider } from './ServerAction/Server.js';
import { ClientActionsConsumer } from './ServerAction/Client.js';

const App = ({ name }: { name: string }) => {
return (
Expand All @@ -12,6 +14,9 @@ const App = ({ name }: { name: string }) => {
<p data-testid="app-name">{name}</p>
<ClientCounter />
<ServerPing />
<ServerProvider>
<ClientActionsConsumer />
</ServerProvider>
</ServerBox>
);
};
Expand Down
12 changes: 12 additions & 0 deletions e2e/fixtures/rsc-basic/src/components/ServerAction/Client.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use client';

import { useActions } from 'ai/rsc';
import { useEffect } from 'react';

export const ClientActionsConsumer = () => {
const actions = useActions();
useEffect(() => {
(globalThis as any).actions = actions;
}, [actions]);
return <div>{JSON.stringify(Object.keys(actions))}</div>;
};
15 changes: 15 additions & 0 deletions e2e/fixtures/rsc-basic/src/components/ServerAction/Server.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { ReactNode } from 'react';
import { createAI } from 'ai/rsc';

const AI = createAI({
actions: {
foo: async () => {
'use server';
return 0;
},
},
});

export function ServerProvider({ children }: { children: ReactNode }) {
return <AI>{children}</AI>;
}
13 changes: 13 additions & 0 deletions e2e/rsc-basic.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,18 @@ for (const { build, command } of commands) {
page.getByTestId('server-ping').getByTestId('counter'),
).toHaveText('2');
});

test('server action', async ({ page }) => {
await page.goto(`http://localhost:${port}/`);
await expect(page.getByTestId('app-name')).toHaveText('Waku');
await expect(page.getByTestId('ai-internal-provider')).toHaveText(
'["foo"]',
);
const result = await page.evaluate(() => {
// @ts-expect-error no types
return globalThis.actions.foo();
});
expect(result).toBe(0);
});
});
}
13 changes: 12 additions & 1 deletion packages/waku/src/lib/plugins/vite-plugin-rsc-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,18 @@ import { parseOpts } from '../utils/swc.js';
const collectExportNames = (mod: swc.Module) => {
const exportNames = new Set<string>();
for (const item of mod.body) {
if (item.type === 'ExportDeclaration') {
if (item.type === 'FunctionDeclaration') {
const rscDeclaration = item.body?.stmts[0];
if (
rscDeclaration?.type === 'ExpressionStatement' &&
rscDeclaration.expression.type === 'StringLiteral' &&
rscDeclaration.expression.value === 'use server'
) {
exportNames.add(item.identifier.value);
}
}
// fixme: this might be incorrect, not all exports from server file can be registered as server references
else if (item.type === 'ExportDeclaration') {
if (item.declaration.type === 'FunctionDeclaration') {
exportNames.add(item.declaration.identifier.value);
} else if (item.declaration.type === 'VariableDeclaration') {
Expand Down
51 changes: 50 additions & 1 deletion packages/waku/src/lib/renderers/rsc-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
import { streamToString } from '../utils/stream.js';
import { decodeActionId } from '../renderers/utils.js';

// HACK for react-server-dom-webpack without webpack
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

(globalThis as any).__webpack_module_loading__ ||= new Map();
(globalThis as any).__webpack_module_cache__ ||= new Map();
(globalThis as any).__webpack_chunk_load__ ||= async (id: string) =>
(globalThis as any).__webpack_module_loading__.get(id);
(globalThis as any).__webpack_require__ ||= (id: string) =>
(globalThis as any).__webpack_module_cache__.get(id);
const moduleLoading = (globalThis as any).__webpack_module_loading__;
const moduleCache = (globalThis as any).__webpack_module_cache__;

export const SERVER_MODULE_MAP = {
'rsdw-server': 'react-server-dom-webpack/server.edge',
'waku-server': 'waku/server',
Expand Down Expand Up @@ -189,7 +199,46 @@
) {
// XXX This doesn't support streaming unlike busboy
const formData = parseFormData(bodyStr, contentType);
args = await decodeReply(formData);
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (!moduleLoading.has(fileId)) {
if (!opts.isDev) {
moduleLoading.set(
fileId,
import(
/* @vite-ignore */
fileId
).then((m: any) => {
moduleCache.set(
fileId,
Object.fromEntries(m['__waku_serverActions']),
);
}),
);
} else {
moduleLoading.set(
fileId,
opts
.loadServerFile(filePathToFileURL(fileId))

Check failure on line 224 in packages/waku/src/lib/renderers/rsc-renderer.ts

View workflow job for this annotation

GitHub Actions / test

Property 'loadServerFile' does not exist on type '{ isDev: true; entries: EntriesDev; loadServerFileRsc: (fileURL: string) => Promise<unknown>; loadServerModuleRsc: (id: string) => Promise<unknown>; resolveClientEntry: (id: string) => string; }'. Did you mean 'loadServerFileRsc'?
.then((m: any) => {
moduleCache.set(
fileId,
Object.fromEntries(m['__waku_serverActions']),
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ugly

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I hope we can improve it.

);
}),
);
}
}
return {
id: fileId,
chunks: [],
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

name,
};
},
});
args = await decodeReply(formData, moduleMap);
} else if (bodyStr) {
args = await decodeReply(bodyStr);
}
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading