Skip to content

Conversation

@cernymatej
Copy link
Member

@cernymatej cernymatej commented Feb 21, 2025

πŸ”— Linked issue

resolves #356

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This will ensure that the build process is terminated when the font face declarations cannot be produced, preventing a project with broken fonts from being deployed.


I'm wondering if this should be addressed upstream in unifont as well.
https://github.com/unjs/unifont/blob/f4d3efae764b78ab4c11cf303e465ecf5b95c1a4/src/index.ts#L55-L57
https://github.com/unjs/unifont/blob/f4d3efae764b78ab4c11cf303e465ecf5b95c1a4/src/index.ts#L79-L81

However, that would likely require a major version bump since it would be a breaking change. Then again, it might not be necessary, as one could simply check the return value to determine whether the resolution was successful.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 82.73%. Comparing base (33e98b0) to head (b81a658).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #518      +/-   ##
==========================================
- Coverage   82.84%   82.73%   -0.11%     
==========================================
  Files          13       13              
  Lines        1067     1066       -1     
  Branches      264      268       +4     
==========================================
- Hits          884      882       -2     
- Misses        178      179       +1     
  Partials        5        5              

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@danielroe
Copy link
Member

It's possible that nuxt/fonts might process something that isn't a valid font (and therefore can't resolve it).

So I think the scenarios we should care about are:

  • if the font providers can't initialise their registries
  • if the font providers don't provide font data even though they have the font in their registry
  • if we can't download the fonts

@cernymatej
Copy link
Member Author

Could you please share an example of a case where something fails to resolve, but we wouldn’t want to throw an error here? πŸ™ I'd like to test it.

The resolveFont function from unifont receives a fontFamily parameter, so I assumed it was already considered a valid font face.

const result = await unifont.resolveFont(fontFamily, defaults, [override.provider])


Are you suggesting that we should throw an error when a failure occurs in these places instead?

Downloading registries
https://github.com/unjs/unifont/blob/f4d3efae764b78ab4c11cf303e465ecf5b95c1a4/src/index.ts#L51

Downloading font details
https://github.com/unjs/unifont/blob/f4d3efae764b78ab4c11cf303e465ecf5b95c1a4/src/index.ts#L71

Downloading fonts

res = await fetch(url).then(r => r.arrayBuffer()).then(r => Buffer.from(r))

(probably unnecessary, since errors aren’t caught here anyway - we can keep the fetch error, if it occurs)

@danielroe
Copy link
Member

danielroe commented Mar 8, 2025

It might fail - I think - if the font family isn't a web font or if it's processing a css variable which doesn't have a font family.

@danielroe
Copy link
Member

we can use unjs/unifont#161 once the next release of unifont

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.

Fonts sometimes missing in production build

3 participants