From d0445996ff7b93e9ded42945700d8c7de214cb44 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 20 Jul 2024 16:28:57 +0100 Subject: [PATCH 1/3] fix: allow `getPlatformProxy` to handle RPC calls returning objects allow the `getPlatformProxy` to handle RPC calls returning objects by making sure that the miniflare magic proxy can take into account methods set via symbols (in the case or RPC objects specifically `Symbol.dispose` and `Symbol.asyncDispose`) additionally add more RPC test coverage in miniflare --- .changeset/polite-dodos-happen.md | 9 ++ .../tests/get-platform-proxy.env.test.ts | 8 +- .../workers/rpc-worker/index.ts | 8 ++ .../src/workers/core/proxy.worker.ts | 10 +- packages/miniflare/test/index.spec.ts | 98 +++++++++++++++++++ 5 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 .changeset/polite-dodos-happen.md diff --git a/.changeset/polite-dodos-happen.md b/.changeset/polite-dodos-happen.md new file mode 100644 index 000000000000..1f9bfef4bc31 --- /dev/null +++ b/.changeset/polite-dodos-happen.md @@ -0,0 +1,9 @@ +--- +"miniflare": patch +--- + +fix: Allow the magic proxy to proxy objects containing functions indexed by symbols + +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 diff --git a/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts b/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts index 3d58b938f955..4bbfabdd5158 100644 --- a/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts +++ b/fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts @@ -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`); diff --git a/fixtures/get-platform-proxy/workers/rpc-worker/index.ts b/fixtures/get-platform-proxy/workers/rpc-worker/index.ts index 540f908c63ac..0d284d9fed94 100644 --- a/fixtures/get-platform-proxy/workers/rpc-worker/index.ts +++ b/fixtures/get-platform-proxy/workers/rpc-worker/index.ts @@ -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; diff --git a/packages/miniflare/src/workers/core/proxy.worker.ts b/packages/miniflare/src/workers/core/proxy.worker.ts index ae400da5ebee..0283af16f59e 100644 --- a/packages/miniflare/src/workers/core/proxy.worker.ts +++ b/packages/miniflare/src/workers/core/proxy.worker.ts @@ -69,7 +69,15 @@ function isPlainObject(value: unknown) { ); } function objectContainsFunctions(obj: Record): boolean { - for (const [, entry] of Object.entries(obj)) { + const propertyNames = Object.getOwnPropertyNames(obj); + const propertySymbols = Object.getOwnPropertySymbols(obj); + const properties = [...propertyNames, ...propertySymbols]; + + 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; } diff --git a/packages/miniflare/test/index.spec.ts b/packages/miniflare/test/index.spec.ts index 302c55cff131..83543452e610 100644 --- a/packages/miniflare/test/index.spec.ts +++ b/packages/miniflare/test/index.spec.ts @@ -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: [ From a454995bb087b5fb647801e1d9a42d8c3dba6df5 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Tue, 30 Jul 2024 11:59:11 +0100 Subject: [PATCH 2/3] avoid @ts-ignore --- packages/miniflare/src/workers/core/proxy.worker.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/miniflare/src/workers/core/proxy.worker.ts b/packages/miniflare/src/workers/core/proxy.worker.ts index 0283af16f59e..f7eb4fb1c742 100644 --- a/packages/miniflare/src/workers/core/proxy.worker.ts +++ b/packages/miniflare/src/workers/core/proxy.worker.ts @@ -68,15 +68,12 @@ function isPlainObject(value: unknown) { Object.getOwnPropertyNames(proto).sort().join("\0") === objectProtoNames ); } -function objectContainsFunctions(obj: Record): boolean { +function objectContainsFunctions(obj: Record): boolean { const propertyNames = Object.getOwnPropertyNames(obj); const propertySymbols = Object.getOwnPropertySymbols(obj); const properties = [...propertyNames, ...propertySymbols]; 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; From 5a4fb43ee61bcfd2b7cdd705aafc90446aa743a9 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:10:44 +0100 Subject: [PATCH 3/3] run prettier --- packages/miniflare/src/workers/core/proxy.worker.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/miniflare/src/workers/core/proxy.worker.ts b/packages/miniflare/src/workers/core/proxy.worker.ts index f7eb4fb1c742..99c3e2db1cb1 100644 --- a/packages/miniflare/src/workers/core/proxy.worker.ts +++ b/packages/miniflare/src/workers/core/proxy.worker.ts @@ -68,7 +68,9 @@ function isPlainObject(value: unknown) { Object.getOwnPropertyNames(proto).sort().join("\0") === objectProtoNames ); } -function objectContainsFunctions(obj: Record): boolean { +function objectContainsFunctions( + obj: Record +): boolean { const propertyNames = Object.getOwnPropertyNames(obj); const propertySymbols = Object.getOwnPropertySymbols(obj); const properties = [...propertyNames, ...propertySymbols];