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: avoid throwing if caches is undeclared #13

Merged
merged 2 commits into from
Mar 2, 2023

Conversation

unkhz
Copy link
Contributor

@unkhz unkhz commented Mar 2, 2023

Problem
Using getAccessToken with a runtime that does not support the global caches property throws a reference error in any runtime that does not support the CacheStorage api. This happens during the support check for the default cache, which refers to the caches directly without having verified the support yet.

Solution
Use globalThis.caches to verify the reference.

@koistya
Copy link
Member

koistya commented Mar 2, 2023

@unkhz thanks, Juhani. Do you think we can use self.caches (for which Cloudflare provides type definitions) instead of globalThis.caches?

@koistya koistya merged commit 8e66008 into kriasoft:main Mar 2, 2023
@unkhz
Copy link
Contributor Author

unkhz commented Mar 2, 2023

I chose globalThis because it should be widely supported. Node 12.0.0+ and Deno 1.0+ supported (along with Bun 0.5.7, which I tested). It's also polyfilled in core-js for more ancient runtimes: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis#browser_compatibility.

Did a quick check and self seems to work nicely for Deno, Bun and Chrome, but throws reference error with Node.

@koistya
Copy link
Member

koistya commented Mar 2, 2023

Good catch! Just published these updates under v1.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants