From 75b4184b3af3e6f67dc0d70c2a0f871807805714 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sat, 27 Jan 2024 00:10:48 +0000 Subject: [PATCH 1/4] expose no-op `caches` in getBindingsProxy --- .changeset/happy-pandas-sparkle.md | 12 +++++ fixtures/get-bindings-proxy/package.json | 3 ++ .../tests/get-bindings-proxy.test.ts | 34 ++++++++++++ .../src/api/integrations/bindings/index.ts | 53 +++++++++++++++++++ pnpm-lock.yaml | 18 ++++--- 5 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 .changeset/happy-pandas-sparkle.md diff --git a/.changeset/happy-pandas-sparkle.md b/.changeset/happy-pandas-sparkle.md new file mode 100644 index 000000000000..a5eef2451500 --- /dev/null +++ b/.changeset/happy-pandas-sparkle.md @@ -0,0 +1,12 @@ +--- +"wrangler": minor +--- + +expose new (no-op) `caches` field in `getBindingsProxy` result + +add a new `caches` field to the `getBindingsProxy` result, such field implements a +no operation (no-op) implementation of the runtime `caches` + +Note: Miniflare exposes a proper `caches` mock, we will want to use that one in +the future but issues regarding it must be ironed out first, so for the +time being a no-op will have to do diff --git a/fixtures/get-bindings-proxy/package.json b/fixtures/get-bindings-proxy/package.json index dd5f5a695774..63e69320b5c6 100644 --- a/fixtures/get-bindings-proxy/package.json +++ b/fixtures/get-bindings-proxy/package.json @@ -11,5 +11,8 @@ "@cloudflare/workers-tsconfig": "workspace:*", "@cloudflare/workers-types": "^4.20221111.1", "wrangler": "workspace:*" + }, + "dependencies": { + "undici": "^5.28.2" } } diff --git a/fixtures/get-bindings-proxy/tests/get-bindings-proxy.test.ts b/fixtures/get-bindings-proxy/tests/get-bindings-proxy.test.ts index a14f7c6ded39..560840f2323d 100644 --- a/fixtures/get-bindings-proxy/tests/get-bindings-proxy.test.ts +++ b/fixtures/get-bindings-proxy/tests/get-bindings-proxy.test.ts @@ -6,6 +6,7 @@ import { Fetcher, R2Bucket, } from "@cloudflare/workers-types"; +import { Request, Response } from "undici"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { getBindingsProxy as originalGetBindingsProxy, @@ -227,6 +228,25 @@ describe("getBindingsProxy", () => { await dispose(); } }); + + describe("caches", () => { + (["default", "named"] as const).forEach((cacheType) => + it(`correctly obtains a no-op ${cacheType} cache`, async () => { + const { caches, dispose } = await getBindingsProxy({ + configPath: wranglerTomlFilePath, + }); + try { + const cache = + cacheType === "default" + ? caches.default + : await caches.open("my-cache"); + testNoOpCache(cache); + } finally { + await dispose(); + } + }) + ); + }); }); /** @@ -263,3 +283,17 @@ async function testDoBinding( const doRespText = await doResp.text(); expect(doRespText).toBe(expectedResponse); } + +async function testNoOpCache( + cache: Awaited>["caches"]["default"] +) { + let match = await cache.match("http://0.0.0.0/test"); + expect(match).toBeUndefined(); + + const req = new Request("http://0.0.0.0/test"); + await cache.put(req, new Response("test")); + const resp = await cache.match(req); + expect(resp).toBeUndefined(); + const deleted = await cache.delete(req); + expect(deleted).toBe(false); +} diff --git a/packages/wrangler/src/api/integrations/bindings/index.ts b/packages/wrangler/src/api/integrations/bindings/index.ts index 1720cc4a36da..64997945a5b9 100644 --- a/packages/wrangler/src/api/integrations/bindings/index.ts +++ b/packages/wrangler/src/api/integrations/bindings/index.ts @@ -39,6 +39,10 @@ export type BindingsProxy> = { * Object containing the various proxies */ bindings: Bindings; + /** + * Caches object emulating the Workers Cache runtime API + */ + caches: CacheStorage; /** * Function used to dispose of the child process providing the bindings implementation */ @@ -83,6 +87,7 @@ export async function getBindingsProxy>( ...vars, ...bindings, }, + caches: getNoopCaches(), dispose: () => mf.dispose(), }; } @@ -162,3 +167,51 @@ function getMiniflarePersistOptions( d1Persist: `${persistPath}/d1`, }; } + +// Note as to why we are re-implementing the Cache types here: +// The Request and Response types to be used with the caches come from miniflare itself, if we +// were to use the proper types users would need to provided to the utility objects typed with +// the actual miniflare types, which is actually what we don't want, so for now we just use +// `unknown`s and we can think of better types later when we actually make the `caches` non no-op +type CacheStorage = { + open(cacheName: string): Promise; + readonly default: Cache; +}; +type CacheRequest = unknown; +type CacheResponse = unknown; + +type Cache = { + delete(request: CacheRequest, options?: CacheQueryOptions): Promise; + match( + request: CacheRequest, + options?: CacheQueryOptions + ): Promise; + put(request: CacheRequest, response: CacheResponse): Promise; +}; +type CacheQueryOptions = { + ignoreMethod?: boolean; +}; + +function getNoopCache(): Cache { + const noopCache: Cache = { + async delete() { + return false; + }, + async match() { + return undefined; + }, + async put() {}, + }; + return noopCache; +} + +// We are not ready to expose miniflare's caches as those are problematic to use in a generic context +// (since they only accept instances of the miniflare Request class and return only instances of the +// miniflare Response class, making them tricky to use in generic node.js code), so we provide a no-op +// implementation here until we sort the above issue out +function getNoopCaches(): CacheStorage { + return { + default: getNoopCache(), + open: () => Promise.resolve(getNoopCache()), + }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f626c0ade075..2b10a20c59df 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.0' +lockfileVersion: '6.1' settings: autoInstallPeers: true @@ -202,6 +202,10 @@ importers: version: link:../../packages/wrangler fixtures/get-bindings-proxy: + dependencies: + undici: + specifier: ^5.28.2 + version: 5.28.2 devDependencies: '@cloudflare/workers-tsconfig': specifier: workspace:* @@ -802,7 +806,7 @@ importers: version: 8.49.0 eslint-config-turbo: specifier: latest - version: 1.10.13(eslint@8.49.0) + version: 1.10.15(eslint@8.49.0) eslint-plugin-import: specifier: 2.26.x version: 2.26.0(@typescript-eslint/parser@6.7.2)(eslint@8.49.0) @@ -10700,13 +10704,13 @@ packages: eslint: 8.49.0 dev: true - /eslint-config-turbo@1.10.13(eslint@8.49.0): - resolution: {integrity: sha512-Ffa0SxkRCPMtfUX/HDanEqsWoLwZTQTAXO9W4IsOtycb2MzJDrVcLmoFW5sMwCrg7gjqbrC4ZJoD+1SPPzIVqg==} + /eslint-config-turbo@1.10.15(eslint@8.49.0): + resolution: {integrity: sha512-76mpx2x818JZE26euen14utYcFDxOahZ9NaWA+6Xa4pY2ezVKVschuOxS96EQz3o3ZRSmcgBOapw/gHbN+EKxQ==} peerDependencies: eslint: '>6.6.0' dependencies: eslint: 8.49.0 - eslint-plugin-turbo: 1.10.13(eslint@8.49.0) + eslint-plugin-turbo: 1.10.15(eslint@8.49.0) dev: false /eslint-import-resolver-node@0.3.7: @@ -11133,8 +11137,8 @@ packages: - typescript dev: true - /eslint-plugin-turbo@1.10.13(eslint@8.49.0): - resolution: {integrity: sha512-el4AAmn0zXmvHEyp1h0IQMfse10Vy8g5Vbg4IU3+vD9CSj5sDbX07iFVt8sCKg7og9Q5FAa9mXzlCf7t4vYgzg==} + /eslint-plugin-turbo@1.10.15(eslint@8.49.0): + resolution: {integrity: sha512-Tv4QSKV/U56qGcTqS/UgOvb9HcKFmWOQcVh3HEaj7of94lfaENgfrtK48E2CckQf7amhKs1i+imhCsNCKjkQyA==} peerDependencies: eslint: '>6.6.0' dependencies: From b53a416b560b807958a2b919759cd92a8fb91f6b Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 30 Jan 2024 11:15:27 +0000 Subject: [PATCH 2/4] Update .changeset/happy-pandas-sparkle.md Co-authored-by: MrBBot --- .changeset/happy-pandas-sparkle.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/happy-pandas-sparkle.md b/.changeset/happy-pandas-sparkle.md index a5eef2451500..3a9417ace9b3 100644 --- a/.changeset/happy-pandas-sparkle.md +++ b/.changeset/happy-pandas-sparkle.md @@ -2,9 +2,9 @@ "wrangler": minor --- -expose new (no-op) `caches` field in `getBindingsProxy` result +feat: expose new (no-op) `caches` field in `getBindingsProxy` result -add a new `caches` field to the `getBindingsProxy` result, such field implements a +Add a new `caches` field to the `getBindingsProxy` result, such field implements a no operation (no-op) implementation of the runtime `caches` Note: Miniflare exposes a proper `caches` mock, we will want to use that one in From 0182c646f50fe9a50e92523d269d9a81931ecd87 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 30 Jan 2024 11:55:16 +0000 Subject: [PATCH 3/4] use classes as suggested in PR review and move caches into its own file --- .../src/api/integrations/bindings/caches.ts | 62 +++++++++++++++++++ .../src/api/integrations/bindings/index.ts | 51 +-------------- 2 files changed, 64 insertions(+), 49 deletions(-) create mode 100644 packages/wrangler/src/api/integrations/bindings/caches.ts diff --git a/packages/wrangler/src/api/integrations/bindings/caches.ts b/packages/wrangler/src/api/integrations/bindings/caches.ts new file mode 100644 index 000000000000..b6a5858285c0 --- /dev/null +++ b/packages/wrangler/src/api/integrations/bindings/caches.ts @@ -0,0 +1,62 @@ +/* eslint-disable unused-imports/no-unused-vars */ + +/** + * Note about this file: + * + * Here we are providing a no-op implementation of the runtime Cache API instead of using + * the miniflare implementation (via `mf.getCaches()`). + * + * We are not using miniflare's implementation because that would require the user to provide + * miniflare-specific Request objects and they would receive back miniflare-specific Response + * objects, this (in particular the Request part) is not really suitable for `getBindingsProxy` + * as people would ideally interact with their bindings in a very production-like manner and + * requiring them to deal with miniflare-specific classes defeats a bit the purpose of the utility. + * + * Similarly the Request and Response types here are set to `undefined` as not to use specific ones + * that would require us to make a choice right now or the user to adapt their code in order to work + * with the api. + * + * We need to find a better/generic manner in which we can reuse the miniflare cache implementation, + * but until then the no-op implementation below will have to do. + */ + +/** + * No-op implementation of CacheStorage + */ +export class CacheStorage { + async open(cacheName: string): Promise { + return new Cache(); + } + + get default(): Cache { + return new Cache(); + } +} + +type CacheRequest = unknown; +type CacheResponse = unknown; + +/** + * No-op implementation of Cache + */ +class Cache { + async delete( + request: CacheRequest, + options?: CacheQueryOptions + ): Promise { + return false; + } + + async match( + request: CacheRequest, + options?: CacheQueryOptions + ): Promise { + return undefined; + } + + async put(request: CacheRequest, response: CacheResponse): Promise {} +} + +type CacheQueryOptions = { + ignoreMethod?: boolean; +}; diff --git a/packages/wrangler/src/api/integrations/bindings/index.ts b/packages/wrangler/src/api/integrations/bindings/index.ts index 64997945a5b9..dcb510321253 100644 --- a/packages/wrangler/src/api/integrations/bindings/index.ts +++ b/packages/wrangler/src/api/integrations/bindings/index.ts @@ -4,6 +4,7 @@ import { getBindings } from "../../../dev"; import { getBoundRegisteredWorkers } from "../../../dev-registry"; import { getVarsForDev } from "../../../dev/dev-vars"; import { buildMiniflareBindingOptions } from "../../../dev/miniflare"; +import { CacheStorage } from "./caches"; import { getServiceBindings } from "./services"; import type { Config } from "../../../config"; import type { MiniflareOptions } from "miniflare"; @@ -87,7 +88,7 @@ export async function getBindingsProxy>( ...vars, ...bindings, }, - caches: getNoopCaches(), + caches: new CacheStorage(), dispose: () => mf.dispose(), }; } @@ -167,51 +168,3 @@ function getMiniflarePersistOptions( d1Persist: `${persistPath}/d1`, }; } - -// Note as to why we are re-implementing the Cache types here: -// The Request and Response types to be used with the caches come from miniflare itself, if we -// were to use the proper types users would need to provided to the utility objects typed with -// the actual miniflare types, which is actually what we don't want, so for now we just use -// `unknown`s and we can think of better types later when we actually make the `caches` non no-op -type CacheStorage = { - open(cacheName: string): Promise; - readonly default: Cache; -}; -type CacheRequest = unknown; -type CacheResponse = unknown; - -type Cache = { - delete(request: CacheRequest, options?: CacheQueryOptions): Promise; - match( - request: CacheRequest, - options?: CacheQueryOptions - ): Promise; - put(request: CacheRequest, response: CacheResponse): Promise; -}; -type CacheQueryOptions = { - ignoreMethod?: boolean; -}; - -function getNoopCache(): Cache { - const noopCache: Cache = { - async delete() { - return false; - }, - async match() { - return undefined; - }, - async put() {}, - }; - return noopCache; -} - -// We are not ready to expose miniflare's caches as those are problematic to use in a generic context -// (since they only accept instances of the miniflare Request class and return only instances of the -// miniflare Response class, making them tricky to use in generic node.js code), so we provide a no-op -// implementation here until we sort the above issue out -function getNoopCaches(): CacheStorage { - return { - default: getNoopCache(), - open: () => Promise.resolve(getNoopCache()), - }; -} From 7fac49f865416d7eefbf3c824d0f40c2e76c88bf Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 30 Jan 2024 12:10:01 +0000 Subject: [PATCH 4/4] make undici a devDependency --- fixtures/get-bindings-proxy/package.json | 4 +--- pnpm-lock.yaml | 9 ++++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/fixtures/get-bindings-proxy/package.json b/fixtures/get-bindings-proxy/package.json index 63e69320b5c6..838544b43098 100644 --- a/fixtures/get-bindings-proxy/package.json +++ b/fixtures/get-bindings-proxy/package.json @@ -10,9 +10,7 @@ "devDependencies": { "@cloudflare/workers-tsconfig": "workspace:*", "@cloudflare/workers-types": "^4.20221111.1", - "wrangler": "workspace:*" - }, - "dependencies": { + "wrangler": "workspace:*", "undici": "^5.28.2" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2b10a20c59df..45c470089867 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.1' +lockfileVersion: '6.0' settings: autoInstallPeers: true @@ -202,10 +202,6 @@ importers: version: link:../../packages/wrangler fixtures/get-bindings-proxy: - dependencies: - undici: - specifier: ^5.28.2 - version: 5.28.2 devDependencies: '@cloudflare/workers-tsconfig': specifier: workspace:* @@ -213,6 +209,9 @@ importers: '@cloudflare/workers-types': specifier: ^4.20221111.1 version: 4.20231025.0 + undici: + specifier: ^5.28.2 + version: 5.28.2 wrangler: specifier: workspace:* version: link:../../packages/wrangler