Skip to content

Replace provider Maps with an internal async cache - #47

Open
jeswr wants to merge 2 commits into
mainfrom
feat/configurable-caches
Open

jeswr wants to merge 2 commits into
mainfrom
feat/configurable-caches

Conversation

@jeswr

@jeswr jeswr commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

On main, CachingIssuerProvider, CachingAuthorizationServerProvider, CachingClientProvider and DPoPTokenProvider each keep a private Map. This PR replaces all four with a private field typed against a small internal async Cache<T> interface (getItem, setItem, removeItem). The interface is shaped like KeyValueKit's KeyValueStore, but KeyValueKit is not added as a dependency. The single implementation, MemoryCache<T>, is backed by a Map and returns the stored object itself. A miss returns null, and the value type excludes null and undefined; this is a type constraint only, not checked at runtime. Every cache operation is now awaited.

Nothing is exposed to callers. Constructors and src/mod.ts exports are unchanged. Cache keys, per-instance caches, the returned objects (including issuer URLs), DPoP refresh and removal order, and the Web Lock scope all behave as on main. Related #45: caller injection and persistent storage are not part of this change. Storage-wide cache keys remain separate in #46.

Validation

  • npm run build (tsc) succeeds.
  • The existing .d.ts files are byte-identical to a build of main, and dist/mod.js is identical too.
  • 13 temporary node:test checks (not committed) ran against the built output with stubbed endpoints. They cover the memory cache, provider hits, misses and retries, and DPoP caching, locking and refresh. All pass on this branch, and the 12 provider checks also pass on main.
Original user request (verbatim)

https://github.com/solid-contrib/reactive-authentication The link provided is to a GitHub repository for a library called Reactive Authentication. The design philosophy behind this library is to reactively upgrade fetch requests when an unauthorized response is returned, and it has been first built for Solid applications. The library, as it stands, is starting to implement caching so that, say issuer metadata, refresh tokens, and depop tokens can be cached and reused to upgrade requests. I would like you to help improve the caching functionality. Specifically, all of the caching at present is done in memory, and the caching is per URL rather than per storage in the context of Solid or LWS. I would like to open an issue, if there is not already one, and I would like you to check if there is already an issue, for both extending in-memory caching to other caching types, including in the browser, using appropriate storages. So the way that I want caches designed is to be configurable. So there's a generic cache interface, and then different components of this library can be instantiated with those caching interfaces. In particular, there's going to be a good browser default, which I imagine you can recommend based on your knowledge of the browser APIs, the credential APIs, the local storage APIs, and which of those APIs are going to be appropriate for caching different types of data that are cached in the library. And then I would like you to open an issue for improving the keying mechanism for caches so that it doesn't always cache just on the request URL, instead it caches to the appropriate context, i.e. storage for linked web storage and Solid. For that storage-wide context, open just an issue at the moment and not a PR. I would like you to look at the existing open PRs and see if they address these issues. Older PRs would have been opened by a less capable LLM to yourself, so you are going to need to start from scratch. But I would like you to reference any PRs that you're superseding and close them as you reference them. Finally, I would like you to include in a fold-out part of the PR description, so it's not there by default, the prompt that I have just given you, so the full context of this conversation is available to the maintainer of the library.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Broad provider, storage, and credential-persistence changes warrant final human review.

Review effort: Lite
Findings: None

What changed in this PR

Adds configurable asynchronous cache adapters and browser persistence across provider components.

Changes:

  • Adds memory, IndexedDB, Web Storage, and TTL caches.
  • Injects caches into issuer, discovery, client, and DPoP providers.
  • Adds browser defaults, persistence tests, smoke tests, and CI coverage.
File Description
test/​providers.test.js Tests provider cache injection and behavior.
test/​dpop-cache.test.js Tests credential persistence and refresh rotation.
test/​cache.test.js Tests adapters, failures, namespaces, and TTLs.
test/​browser-cache.html Tests browser persistence and isolation.
src/​WebStorageCache.ts Implements codec-based Web Storage caching.
src/​mod.ts Exports cache APIs and adapters.
src/​MemoryCache.ts Implements in-memory caching.
src/​IndexedDbCache.ts Implements IndexedDB persistence.
src/​ExpiringCache.ts Adds absolute-TTL behavior.
src/​DPoPTokenProvider.ts Adds injectable credential caching and rotation handling.
src/​createBrowserCaches.ts Provides browser cache defaults.
src/​CachingIssuerProvider.ts Adds injectable issuer caching.
src/​CachingClientProvider.ts Adds injectable client caching.
src/​CachingAuthorizationServerProvider.ts Adds injectable discovery caching.
src/​Cache.ts Defines the asynchronous cache contract.
README.md Documents cache configuration and browser storage policy.
package.json Adds test tooling and dependencies.
.github/​workflows/​ci.yml Runs the expanded test suite.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jeswr

jeswr commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

@langsamu note that I have taken a brief look at this PR. Overall I think it is good.

I was curious as to whether there is a good library out there that we can use for the generic caching interface.

The recommendation from Astra was to use idb-keyval to cleanup the IndexDB adapter logic; whilst maintaining the rest of the logic ourselves for now. What is your appetite for introducing idb-keyval as a dependency?

The full response can be viewed here.

EDIT: It turns out you can synthesise a library with exactly the functionality we want first shot with Astra. See it available here. Note the interface uses getItem, setItem as methods to align with localForage and unstorage interfaces. I'd suggest we use that library as a dependency and remove all caching logic from here. This is what the PR would like like if we did so

@jeswr

This comment was marked as resolved.

@jeswr jeswr closed this Sep 25, 2026
@jeswr jeswr reopened this Sep 25, 2026
@jeswr jeswr changed the title Add configurable provider caches and browser storage adapters Replace provider Maps with an internal async cache Sep 29, 2026

This branch has not been deployed

No deployments
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