Skip to content

fix(cache): isolate cache keys per provider name and options#281

Merged
danielroe merged 9 commits intounjs:mainfrom
sushichan044:sushichan044-202511081110-provider-aware
Nov 21, 2025
Merged

fix(cache): isolate cache keys per provider name and options#281
danielroe merged 9 commits intounjs:mainfrom
sushichan044:sushichan044-202511081110-provider-aware

Conversation

@sushichan044
Copy link
Contributor

@sushichan044 sushichan044 commented Nov 10, 2025

fixes #184

This PR updates the cache storage generation to automatically include the provider name and options in the cache key.
Users don’t need to do anything special when defining a provider.

Additional Context

Purpose 1 of #239 (comment).

@sushichan044 sushichan044 changed the title fix(cache): auto include provier's name and options to isolate namespace fix(cache): automatically isolate cache namespace based on provider name and options Nov 10, 2025
@sushichan044 sushichan044 changed the title fix(cache): automatically isolate cache namespace based on provider name and options fix(cache): Isolate cache namespace based on provider name and options Nov 10, 2025
@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from 85c0863 to fa869d3 Compare November 10, 2025 15:48
@sushichan044 sushichan044 marked this pull request as ready for review November 10, 2025 15:57
@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from cb6bacd to d30e722 Compare November 10, 2025 16:05
@sushichan044 sushichan044 changed the title fix(cache): Isolate cache namespace based on provider name and options fix(cache): isolate cache namespace based on provider name and options Nov 10, 2025
@sushichan044 sushichan044 changed the title fix(cache): isolate cache namespace based on provider name and options fix(cache): isolate cache keys per provider name and options Nov 16, 2025
@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from 0966299 to f10ed13 Compare November 20, 2025 11:47
src/cache.ts Outdated
Comment on lines 75 to 76
// No hash for string parts for better readability.
const part = typeof f === 'string' ? f : hash(f)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example, if we also hash values like the font provider’s name, it becomes difficult to identify files when checking what Storage has saved. So I think we shouldn’t hash string fields.

@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from e1d8f29 to 9f764a1 Compare November 20, 2025 12:54
@sushichan044
Copy link
Contributor Author

sushichan044 commented Nov 20, 2025

@danielroe
I think this implementation safely isolates the cache between providers, noted in #239.
I’m still keeping an eye on the changes being discussed in #287 and #288.
Could you review the design?

@sushichan044
Copy link
Contributor Author

Since the commit history became too cluttered and conflicted with other PRs, I used a force push.

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

apologies for going back and forth on this.

we should definitely cache by key (font provider) + version of unifont.

my one reservation is that caching uniquely by options too, may be over-unique. for example, google fonts will have the same list of fonts, regardless of what options are passed to it. so I'd like to refactor later on to allow providers a little more control over what is dependent on the options and what isn't...

however, I think this is looking really good, and I don't want to hold it up, so I think we can merge and iterate (if you're happy to address the merge conflict 🙏

thank you for your work on this

@sushichan044
Copy link
Contributor Author

sushichan044 commented Nov 21, 2025

Thanks!
I’ll watch the other PRs and keep iterating on improvements after this one is merged.

@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from 9f764a1 to 1113b6f Compare November 21, 2025 14:15
@sushichan044
Copy link
Contributor Author

sushichan044 commented Nov 21, 2025

Conflict resolved in https://github.com/unjs/unifont/compare/9f764a10cd601db0add398a6f730db8cbb7cd5f0..1113b6febd3f96d2b4b5d3c69ad180e8f22f642d

This diff is safe because it doesn’t affect any cache-related test cases and CI passes without differences.

@sushichan044 sushichan044 force-pushed the sushichan044-202511081110-provider-aware branch from d8b20f2 to adf9920 Compare November 21, 2025 14:52
@danielroe danielroe merged commit b8d8635 into unjs:main Nov 21, 2025
4 checks passed
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.

Cache does not take provider config into account

2 participants