refactor!: make the provider fetcher internal by florian-lefebvre · Pull Request #489 · unjs/unifont · GitHub

Removes fetch from ProviderContext and keeps it as an internal concern. It is an implementation detail of built-in providers.

When working on withastro/astro#17830 I found this new thing which would leak in Astro. I could workaround it in Astro but I figured it'd be better to fix in unifont directly.

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Clarified that custom providers use the browser’s standard network requests.
- Custom provider APIs must support CORS or be accessed through a proxy in browser-based applications.
- Clarified that API base URL rewriting applies only to built-in providers.
Improvements
- Standardized network requests across built-in font and icon providers while preserving existing retrieval behavior.
- Cached storage behavior remains unchanged.

`ctx.fetch` is no longer part of `ProviderContext`. The `apiBase`-aware fetcher is now attached to the context under an internal symbol and reached through `fetchAPI(ctx, url, init)`, which only the built-in providers use. Custom providers request their own endpoints with the global `fetch`; those URLs were never rewritten to `apiBase` anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Fi53GTT12xDNYxQLQAdc7

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
unifont-proxy	Ready	Preview	Aug 26, 2026 1:19pm
unifont.dev	Ready	Preview	Aug 26, 2026 1:19pm

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9e3ee030-705f-4c78-bc9f-4f7b946d83ae

📥 Commits

Reviewing files that changed from the base of the PR and between ce39a5e and 0c5c7fd.

📒 Files selected for processing (12)

docs/content/docs/8.custom-providers.md
src/api-base.ts
src/internal.ts
src/providers/adobe.ts
src/providers/bunny.ts
src/providers/fontshare.ts
src/providers/fontsource.ts
src/providers/google.ts
src/providers/googleicons.ts
src/providers/npm.ts
src/types.ts
src/unifont.ts

💤 Files with no reviewable changes (1)

src/types.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

📝 Walkthrough

Walkthrough

Changes

API Fetch Routing

Layer / File(s)	Summary
Fetch contract and provider wiring src/types.ts, src/internal.ts, src/api-base.ts, src/unifont.ts	The public context no longer exposes fetch. An internal APIFetch contract and API_FETCH context key now route provider API requests.
Built-in provider request migration src/providers/*.ts	Built-in providers use fetchAPI for metadata, stylesheet, catalog, and CDN requests. Request handling remains unchanged.
Custom provider documentation docs/content/docs/8.custom-providers.md	Custom providers now use global fetch. The documentation limits apiBase rewriting to built-in provider URLs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0c5c7

The PR makes the provider fetcher internal without any identified current-head correctness, security, availability, or deployment risk; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Built-in provider
  participant fetchAPI
  participant InternalProviderContext
  participant apiFetch
  Built-in provider->>fetchAPI: Request provider API URL
  fetchAPI->>InternalProviderContext: Read API_FETCH
  InternalProviderContext->>apiFetch: Forward URL and request options
  apiFetch-->>Built-in provider: Return Response

Suggested reviewers: danielroe

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: moving the provider fetcher from the public context to an internal implementation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 10 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1 📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch refactor/internal-provider-fetch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (ce39a5e) to head (0c5c7fd).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #489   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        19    +1     
  Lines         1142      1144    +2     
  Branches       285       285           
=========================================
+ Hits          1142      1144    +2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

this is meant to be a useful tool for providers to use in the same way that storage is- it handles retries (and replaced our old fetchWithRetries function) - and might do more in the future ...

if you want to drop it at the type leve, you could do Omit<UnifontOptions>?

or do you have another concern about exposing it on the ctx?

I'm afraid of scope creep for fetch(). For example the proxy makes sense...but only for the buit-in providers. Yet it's part of a public API.

So removing may not be the right solution, I think retries are nice to have.

Do you think it'd be acceptable to make the proxy logic internal instead?

what do you mean the proxy is part of a public API? it's internal code, not exposed via a user option, it only applies if the apiBase is set, and the URL being fetched is one of these:

unifont/src/api-base.ts

Lines 15 to 25 in 153e39e

    
           const proxyRoutes: [pattern: RegExp, route: string][] = [ 
        
             [/^https:\/\/typekit\.com\/api\/v1\/json\/kits\/([^/]+)\/published$/, '/adobe/v1/kit/$1'], 
        
             [/^https:\/\/use\.typekit\.net\/([^/]+)\.css$/, '/adobe/v1/kit-css/$1'], 
        
             [/^https:\/\/fonts\.bunny\.net\//, '/bunny/v1/'], 
        
             [/^https:\/\/api\.fontshare\.com\/v2\//, '/fontshare/v1/'], 
        
             [/^https:\/\/api\.fontsource\.org\/v1\//, '/fontsource/v1/'], 
        
             [/^https:\/\/fonts\.google\.com\/metadata\/fonts/, '/google/v1/fonts'], 
        
             [/^https:\/\/fonts\.google\.com\/metadata\/icons/, '/google/v1/icons'], 
        
             [/^https:\/\/fonts\.googleapis\.com\/css2/, '/google/v1/css'], 
        
             [/^https:\/\/fonts\.googleapis\.com\/icon/, '/google/v1/icon'], 
        
           ]

we could remove it and there would be no breaking change for consumers, so I regard it as internal.

(I'm more than happy to refactor how the proxy works; as it's not a breaking change we can do so at any time. the reason it's built the way it is, is just that I wanted to have one source of truth rather than doing so in each provider)

vercel Bot deployed to Preview – unifont-proxy August 26, 2026 13:19 View deployment

vercel Bot deployed to Preview – unifont.dev August 26, 2026 13:19 View deployment

florian-lefebvre mentioned this pull request Aug 26, 2026

feat(fonts): warn on invalid properties withastro/astro#17830

Draft

florian-lefebvre marked this pull request as ready for review August 26, 2026 13:43

florian-lefebvre requested a review from danielroe as a code owner August 26, 2026 13:43

danielroe reviewed Aug 26, 2026

View reviewed changes

refactor!: make the provider fetcher internal by florian-lefebvre · Pull Request #489 · unjs/unifont · GitHub

Uh oh!

Uh oh!

refactor!: make the provider fetcher internal - #489

refactor!: make the provider fetcher internal#489
florian-lefebvre wants to merge 1 commit into
mainfrom
refactor/internal-provider-fetch

florian-lefebvre commented Aug 26, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

vercel Bot commented Aug 26, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 26, 2026 •
edited
Loading

Uh oh!

Uh oh!

codecov Bot commented Aug 26, 2026

Uh oh!

danielroe left a comment •
edited
Loading

Uh oh!

Uh oh!

florian-lefebvre commented Aug 27, 2026

Uh oh!

danielroe commented Aug 27, 2026 •
edited
Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor!: make the provider fetcher internal by florian-lefebvre · Pull Request #489 · unjs/unifont · GitHub

Uh oh!

Uh oh!

refactor!: make the provider fetcher internal - #489

refactor!: make the provider fetcher internal#489florian-lefebvre wants to merge 1 commit intomainunjs/unifont:mainfrom refactor/internal-provider-fetchunjs/unifont:refactor/internal-provider-fetchCopy head branch name to clipboard

Conversation

florian-lefebvre commented Aug 26, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

vercel Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Aug 26, 2026

Codecov Report

Uh oh!

danielroe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

florian-lefebvre commented Aug 27, 2026

Uh oh!

danielroe commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

refactor!: make the provider fetcher internal#489
florian-lefebvre wants to merge 1 commit into
mainfrom
refactor/internal-provider-fetch

florian-lefebvre commented Aug 26, 2026 •
edited by coderabbitai Bot
Loading

vercel Bot commented Aug 26, 2026 •
edited
Loading

coderabbitai Bot commented Aug 26, 2026 •
edited
Loading

danielroe left a comment •
edited
Loading

danielroe commented Aug 27, 2026 •
edited
Loading