Skip to content

Conversation

@florian-lefebvre
Copy link
Member

Changes

  • Closes Font API: Only one font loading #13637
  • Unifont deduplicates providers based on the provider name. However in our case the same provider can be called with a different config (eg. adobe), so we have some logic to hash the config and dedupe
  • However, the logic AND the tests were wrong because in certain cases, the family provider name was not assigned a value and as such, the family could not be resolved by unifont

Testing

Tests updated + manual

Docs

Changeset

@florian-lefebvre florian-lefebvre self-assigned this Apr 17, 2025
@changeset-bot
Copy link

changeset-bot bot commented Apr 17, 2025

🦋 Changeset detected

Latest commit: 2eae39d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Apr 17, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 17, 2025

CodSpeed Performance Report

Merging #13639 will not alter performance

Comparing fix/fonts-providers-dedupe (2eae39d) with main (d475afc)

Summary

✅ 6 untouched benchmarks

Comment on lines -336 to -338
if (hashes.has(hash)) {
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused here. Isn't the logic here unchanged? If the hash isn't in the hashes set, add the hash to the set and push the provider into the list?

Copy link
Member Author

Choose a reason for hiding this comment

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

What changes is that provider.name is always updated now

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't get that it was mutating the providers you were passing in

@florian-lefebvre florian-lefebvre merged commit 23410c6 into main Apr 17, 2025
16 checks passed
@florian-lefebvre florian-lefebvre deleted the fix/fonts-providers-dedupe branch April 17, 2025 10:53
@astrobot-houston astrobot-houston mentioned this pull request Apr 17, 2025
openscript pushed a commit to openscript/astro that referenced this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Font API: Only one font loading

2 participants