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

fix: allow getPlatformProxy to handle RPC calls returning objects #6301

Merged
merged 3 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
9 changes: 9 additions & 0 deletions .changeset/polite-dodos-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"miniflare": patch
---

fix: Allow the magic proxy to proxy objects containing functions indexed by symbols
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved

In https://github.com/cloudflare/workers-sdk/pull/5670 we introduced the possibility
of the magic proxy to handle object containing functions, the implementation didn't
account for functions being indexed by symbols, address such issue
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,15 @@ describe("getPlatformProxy - env", () => {
rpc = env.MY_RPC as unknown as EntrypointService;
return dispose;
});
it("can call RPC methods directly", async () => {
it("can call RPC methods returning a string", async () => {
expect(await rpc.sum([1, 2, 3])).toMatchInlineSnapshot(`6`);
});
it("can call RPC methods returning an object", async () => {
expect(await rpc.sumObj([1, 2, 3, 5])).toEqual({
isObject: true,
value: 11,
});
});
it("can call RPC methods returning a Response", async () => {
const resp = await rpc.asJsonResponse([1, 2, 3]);
expect(resp.status).toMatchInlineSnapshot(`200`);
Expand Down
8 changes: 8 additions & 0 deletions fixtures/get-platform-proxy/workers/rpc-worker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ export class NamedEntrypoint extends WorkerEntrypoint {
sum(args: number[]): number {
return args.reduce((a, b) => a + b);
}

sumObj(args: number[]): { isObject: true; value: number } {
return {
isObject: true,
value: args.reduce((a, b) => a + b),
};
}

asJsonResponse(args: unknown): {
status: number;
text: () => Promise<string>;
Expand Down
10 changes: 9 additions & 1 deletion packages/miniflare/src/workers/core/proxy.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,15 @@ function isPlainObject(value: unknown) {
);
}
function objectContainsFunctions(obj: Record<string, unknown>): boolean {
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved
for (const [, entry] of Object.entries(obj)) {
const propertyNames = Object.getOwnPropertyNames(obj);
const propertySymbols = Object.getOwnPropertySymbols(obj);
const properties = [...propertyNames, ...propertySymbols];
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved

for (const property of properties) {
// @ts-ignore - ignoring the following line since TypeScript seems to
// incorrectly error if `property` is a symbol
// (see: https://github.com/Microsoft/TypeScript/issues/24587)
const entry = obj[property];
if (typeof entry === "function") {
return true;
}
Expand Down
98 changes: 98 additions & 0 deletions packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,104 @@ test("Miniflare: service binding to named entrypoint", async (t) => {
});
});

test("Miniflare: service binding to named entrypoint that implements a method returning a plain object", async (t) => {
const mf = new Miniflare({
workers: [
{
name: "a",
serviceBindings: {
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
},
compatibilityFlags: ["rpc"],
modules: true,
script: `
export default {
async fetch(request, env) {
const obj = await env.RPC_SERVICE.getObject();
return Response.json({ obj });
}
}
`,
},
{
name: "b",
modules: true,
script: `
import { WorkerEntrypoint } from "cloudflare:workers";
export class RpcEntrypoint extends WorkerEntrypoint {
getObject() {
return {
isPlainObject: true,
value: 123,
}
}
}
`,
},
],
});
t.teardown(() => mf.dispose());

const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
const o = await bindings.RPC_SERVICE.getObject();
t.deepEqual(o.isPlainObject, true);
t.deepEqual(o.value, 123);
});

test("Miniflare: service binding to named entrypoint that implements a method returning an RpcTarget instance", async (t) => {
const mf = new Miniflare({
workers: [
{
name: "a",
serviceBindings: {
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
},
compatibilityFlags: ["rpc"],
modules: true,
script: `
export default {
async fetch(request, env) {
const rpcTarget = await env.RPC_SERVICE.getRpcTarget();
return Response.json(rpcTarget.id);
}
}
`,
},
{
name: "b",
modules: true,
script: `
import { WorkerEntrypoint, RpcTarget } from "cloudflare:workers";

export class RpcEntrypoint extends WorkerEntrypoint {
getRpcTarget() {
return new SubService("test-id");
}
}

class SubService extends RpcTarget {
#id

constructor(id) {
super()
this.#id = id
}

get id() {
return this.#id
}
}
`,
},
],
});
t.teardown(() => mf.dispose());

const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
const rpcTarget = await bindings.RPC_SERVICE.getRpcTarget();
t.deepEqual(rpcTarget.id, "test-id");
});

test("Miniflare: custom outbound service", async (t) => {
const mf = new Miniflare({
workers: [
Expand Down
Loading