Skip to content

Commit

Permalink
Improve getPlatformProxy's (no-op) caches (#5026)
Browse files Browse the repository at this point in the history
* [wrangler] fix: make sure `getPlatformProxy` produces a production-like `caches` object

* [wrangler] fix: relax the `getPlatformProxy`'s' cache request/response types
  • Loading branch information
dario-piotrowicz authored Feb 16, 2024
1 parent efe8b44 commit 0458472
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 2 deletions.
12 changes: 12 additions & 0 deletions .changeset/cuddly-icons-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": patch
---

fix: make sure `getPlatformProxy` produces a production-like `caches` object

make sure that the `caches` object returned to `getPlatformProxy` behaves
in the same manner as the one present in production (where calling unsupported
methods throws a helpful error message)

note: make sure that the unsupported methods are however not included in the
`CacheStorage` type definition
11 changes: 11 additions & 0 deletions .changeset/rich-needles-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"wrangler": patch
---

fix: relax the `getPlatformProxy`'s' cache request/response types

prior to these changes the caches obtained from `getPlatformProxy`
would use `unknown`s as their types, this proved too restrictive
and incompatible with the equivalent `@cloudflare/workers-types`
types, we decided to use `any`s instead to allow for more flexibility
whilst also making the type compatible with workers-types
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,44 @@ describe("getPlatformProxy - caches", () => {
}
})
);

it("should match the production runtime caches object", async () => {
const { caches: platformProxyCaches, dispose } = await getPlatformProxy();
const caches = platformProxyCaches as any;
try {
expect(Object.keys(caches)).toEqual(["default"]);

expect(() => {
caches.has("my-cache");
}).toThrowError(
"Failed to execute 'has' on 'CacheStorage': the method is not implemented."
);

expect(() => {
caches.delete("my-cache");
}).toThrowError(
"Failed to execute 'delete' on 'CacheStorage': the method is not implemented."
);

expect(() => {
caches.keys();
}).toThrowError(
"Failed to execute 'keys' on 'CacheStorage': the method is not implemented."
);

expect(() => {
caches.match(new URL("https://localhost"));
}).toThrowError(
"Failed to execute 'match' on 'CacheStorage': the method is not implemented."
);

expect(() => {
caches.nonExistentMethod();
}).toThrowError("caches.nonExistentMethod is not a function");
} finally {
await dispose();
}
});
});

async function testNoOpCache(
Expand Down
28 changes: 26 additions & 2 deletions packages/wrangler/src/api/integrations/platform/caches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,24 @@
* No-op implementation of CacheStorage
*/
export class CacheStorage {
constructor() {
const unsupportedMethods = ["has", "delete", "keys", "match"];
unsupportedMethods.forEach((method) => {
Object.defineProperty(this, method, {

Check warning on line 30 in packages/wrangler/src/api/integrations/platform/caches.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/api/integrations/platform/caches.ts#L27-L30

Added lines #L27 - L30 were not covered by tests
enumerable: false,
value: () => {
throw new Error(

Check warning on line 33 in packages/wrangler/src/api/integrations/platform/caches.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/api/integrations/platform/caches.ts#L32-L33

Added lines #L32 - L33 were not covered by tests
`Failed to execute '${method}' on 'CacheStorage': the method is not implemented.`
);
},
});
});
Object.defineProperty(this, "default", {

Check warning on line 39 in packages/wrangler/src/api/integrations/platform/caches.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/api/integrations/platform/caches.ts#L39

Added line #L39 was not covered by tests
enumerable: true,
value: this.default,
});
}

async open(cacheName: string): Promise<Cache> {
return new Cache();
}
Expand All @@ -33,8 +51,14 @@ export class CacheStorage {
}
}

type CacheRequest = unknown;
type CacheResponse = unknown;
/* eslint-disable @typescript-eslint/no-explicit-any --
In order to make the API convenient to use in and Node.js programs we try not to
restrict the types that's why we're using `any`s as the request/response types
(making this API flexible and compatible with the cache types in `@cloudflare/workers-types`)
*/
type CacheRequest = any;
type CacheResponse = any;
/* eslint-enable @typescript-eslint/no-explicit-any */

/**
* No-op implementation of Cache
Expand Down

0 comments on commit 0458472

Please sign in to comment.