fix: loading fonts performance issue #170
Merged
Conversation
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
gustavobelo-dev
requested review from
a team,
Kirpal,
btrautmann,
jeroen-meijer,
jolexxa,
marcossevilla and
samandmoore
as code owners
Collaborator
|
domain lgtm |
btrautmann
approved these changes
Nov 5, 2025
Contributor
btrautmann
left a comment
btrautmann
left a comment
There was a problem hiding this comment.
domain lgtm 🚀
platform lgtm 💪
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
There is an issue related to load fonts api when we have several golden tests. In my project we created golden tests to more than 300 icons, and it was not possible to run because the package crashes with out of memory.
I analyze and validate that it has an issue related to load fonts repeatedly, so, I created a local register about fonts already loaded. I tested this solution with multiple tests and files and it works well. I added an example test of the issue but flags as skipped to avoid generating hundreds of files unnecessarily.
I know that another PR try to externalize the loadFonts method and it was suggested to cache the information could be better, so I try it.
Video running the current version of the package:
https://github.com/user-attachments/assets/624b05ef-f06b-4e61-9b88-0e78fb0faa8c
Video running the PR version:
https://github.com/user-attachments/assets/a866e46a-285f-4c8e-80b4-5f68789efd48
If you prefer, I can made this implementation optional but I believe that can be a necessary solution.
Type of Change