feat: getAvailableFontProperties()#398
Open
florian-lefebvre wants to merge 13 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds a new getAvailableFontProperties() method to the Unifont API, enabling callers to query available font properties (formats, styles, weights, subsets) from a provider stack. The method is implemented in the core orchestration layer and supported by all font providers with appropriate metadata introspection. Fontsource includes caching of variable-font axes. Comprehensive documentation and test coverage are included. ChangesAdd getAvailableFontProperties capability
Sequence DiagramsequenceDiagram
participant Client
participant Unifont
participant Provider1
participant Provider2
Client->>Unifont: getAvailableFontProperties(fontFamily)
Unifont->>Provider1: getAvailableFontProperties(fontFamily)
alt Provider1 has method
Provider1-->>Unifont: properties or undefined
alt Result found
Unifont-->>Client: properties + provider name
else Result empty
Unifont->>Provider2: getAvailableFontProperties(fontFamily)
Provider2-->>Unifont: properties or undefined
Unifont-->>Client: properties + provider name or undefined
end
else Provider1 no method or throws
Unifont->>Provider2: getAvailableFontProperties(fontFamily)
Provider2-->>Unifont: properties or undefined
Unifont-->>Client: properties + provider name or undefined
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #398 +/- ##
==========================================
+ Coverage 98.37% 98.51% +0.14%
==========================================
Files 12 12
Lines 676 741 +65
Branches 176 193 +17
==========================================
+ Hits 665 730 +65
Misses 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
README.md (1)
659-659: 💤 Low valueConsider documenting the type signature consistently with other methods.
The type signature uses
providers?: T[number]['_name'][], whileresolveFont()(line 484) andlistFonts()(line 624) documentproviders?: T[]. WhileT[number]['_name'][]is more precise (it's explicitly the array of provider name strings), this inconsistency might confuse users comparing the API methods.The Providers subsection correctly specifies
Type: string[], so the usage is clear, but consider aligning the method signature format with existing methods for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 659, Update the documented Type signature so it matches the format used by resolveFont() and listFonts(): replace "providers?: T[number]['_name'][]" with "providers?: T[]" in the Type line for the GetAvailableFontProperties method (the line currently showing "(fontFamily: string, providers?: T[number]['_name'][]) => Promise<GetAvailableFontPropertiesResult & { provider?: T[number]['_name'] }") to keep method docs consistent with the Providers subsection and the other method signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 689: Rename the misleading variable to reflect that
getAvailableFontProperties() returns properties rather than names: replace
occurrences of availableFonts with a clearer identifier like
availableFontProperties (or fontProperties) in the example using
getAvailableFontProperties('Roboto', ['google']) so the variable name aligns
with the method purpose and avoids confusion.
In `@src/providers/fontsource.ts`:
- Around line 95-111: In getAvailableFontProperties, a rejection from
getVariableAxes when font.variable is true will abort the whole call; wrap the
await getVariableAxes(ctx, font) in a try/catch so failures degrade gracefully:
attempt the call only when font.variable, and on error swallow or log it and
continue returning the static properties (formats, styles, subsets, weights)
without variable wght range; only push the `${variableAxes.axes.wght.min}
${variableAxes.axes.wght.max}` string if variableAxes was successfully retrieved
and variableAxes.axes.wght exists.
In `@src/providers/googleicons.ts`:
- Around line 87-97: getAvailableFontProperties currently returns weights
['100','400'] for non-Icons families which contradicts the Material Symbols
usage that fetches wght over 100..700; update getAvailableFontProperties (and
keep reference to googleIcons and the fontFamily param) so that non-Icons
families return the full weight range
['100','200','300','400','500','600','700'] while preserving Icons families as
['400'] to remain consistent with wght handling and downstream
validation/suggestions.
In `@src/unifont.ts`:
- Line 23: The getAvailableFontProperties implementation incorrectly treats an
empty object returned from a provider as a successful hit (because {} is truthy)
and also fails to attach the provider name to the success result; update the
provider loop inside getAvailableFontProperties to explicitly detect an
empty/invalid result (e.g., check for required keys like variants, subsets,
metrics or Object.keys(result).length === 0) and continue to the next provider
when empty, and when a non-empty result is found, return the result augmented
with provider: currentProviderName so the return type { provider?: ... } is
satisfied.
---
Nitpick comments:
In `@README.md`:
- Line 659: Update the documented Type signature so it matches the format used
by resolveFont() and listFonts(): replace "providers?: T[number]['_name'][]"
with "providers?: T[]" in the Type line for the GetAvailableFontProperties
method (the line currently showing "(fontFamily: string, providers?:
T[number]['_name'][]) => Promise<GetAvailableFontPropertiesResult & { provider?:
T[number]['_name'] }") to keep method docs consistent with the Providers
subsection and the other method signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ae09ba22-65a9-41af-8544-3b27385789e2
📒 Files selected for processing (15)
README.mdsrc/index.tssrc/providers/bunny.tssrc/providers/fontshare.tssrc/providers/fontsource.tssrc/providers/google.tssrc/providers/googleicons.tssrc/types.tssrc/unifont.tstest/index.test.tstest/providers/bunny.test.tstest/providers/fontshare.test.tstest/providers/fontsource.test.tstest/providers/google.test.tstest/providers/googleicons.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/providers/bunny.ts (1)
73-73: ⚡ Quick winMake
subsetsordering deterministic before returning it.Line 73 currently returns subset keys in upstream metadata order, which can drift and create unstable API output/snapshots. Sorting here makes responses deterministic for consumers.
Proposed patch
- subsets: Object.keys(font.variants), + subsets: Object.keys(font.variants).sort(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/bunny.ts` at line 73, The returned subsets array is currently using Object.keys(font.variants) which preserves upstream order and can be unstable; change the assignment for subsets to a deterministic sorted list by calling .sort() (preferably .sort((a,b)=>a.localeCompare(b))) on the Object.keys(...) result so the response/snapshots are stable; update the expression that sets subsets in the code that references font.variants accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/providers/bunny.ts`:
- Line 73: The returned subsets array is currently using
Object.keys(font.variants) which preserves upstream order and can be unstable;
change the assignment for subsets to a deterministic sorted list by calling
.sort() (preferably .sort((a,b)=>a.localeCompare(b))) on the Object.keys(...)
result so the response/snapshots are stable; update the expression that sets
subsets in the code that references font.variants accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d179af2-43ee-4364-93d2-19d05c665bc4
📒 Files selected for processing (2)
src/providers/bunny.tstest/providers/bunny.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new
getAvailableFontProperties()API to the unifont instance and providers. Currently when you pass options toresolveFont()you have no way to know if what you're providing can actually be used by the provider. This new API allows such thing now. Examples:Tests have been added for all. Note that I didn't implement it for 2 providers:
Readme is updated
Summary by CodeRabbit
New Features
Documentation
Tests