-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Feat/precompile fonts #3003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feat/precompile fonts #3003
Conversation
## [11.2.1](infinitered/ignite@v11.2.0...v11.2.1) (2025-10-06) ### Bug Fixes * **boilerplate:** update worklets exclude to drop version ([infinitered#3001](infinitered#3001) by [@frankcalise](https://github.com/frankcalise)) ([3e8c37e](infinitered@3e8c37e))
…OS and Android Fixes infinitered#2995
|
Also I apologize, I somehow back tracked a release ci PR.. my bad. Idk how to undo that part. |
coolsoftwaretyler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ChristopherGabba - thanks again for the PR. It's good timing because we've been discussing this kind of thing internally for a couple weeks now.
@frankcalise did find a different way to handle fully cross platform functionality with a more uniform API. Do you want to take a look at the example I posted and see if you can adopt that pattern instead?
boilerplate/app/app.tsx
Outdated
| // We load fonts dynamically for web only, the rest are handled by | ||
| // the expo-font config plugin in `app.json`. If not using web, | ||
| // you can delete this permissive check along with associated | ||
| // code in `typography'. | ||
| const [areFontsLoadedWebOnly, fontLoadErrorWebOnly] = useFonts(customFontsToLoadWebOnly) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: we had been talking about this internally for a bit before your PR and were looking at an approach similar to how Evan Bacon and Expo are handling fonts cross platform. Rather than deleting for web, I think it would be ideal to gracefully handle web without having users make a choice.
One example @frankcalise found was this AsyncFont component from Evan: https://github.com/EvanBacon/expo-router-forms-components/blob/main/src/components/data/async-font.tsx
Used like this:
I'd prefer that implementation, myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I wouldn't have come up with that approach, no matter how much time I was given, I haven't studied the React 19 APIs very much.
This would probably include moving the i18n load permissive to the suspense fallback as well right? You want me to pursue that approach and see where it leads?
boilerplate/app.json
Outdated
| [ | ||
| "expo-font", | ||
| { | ||
| "fonts": [ | ||
| "./assets/fonts/SpaceGrotesk-300Light.ttf", | ||
| "./assets/fonts/SpaceGrotesk-400Regular.ttf", | ||
| "./assets/fonts/SpaceGrotesk-500Medium.ttf", | ||
| "./assets/fonts/SpaceGrotesk-600SemiBold.ttf", | ||
| "./assets/fonts/SpaceGrotesk-700Bold.ttf" | ||
| ] | ||
| } | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: thank you for this! Really appreciate your time wiring all this up.
package.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "version": "11.2.0", |
suggestion: no need for this, I think CI will do it for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: let's undo this change before merge, please.
boilerplate/app/theme/typography.ts
Outdated
| medium: "spaceGroteskMedium", | ||
| semiBold: "spaceGroteskSemiBold", | ||
| bold: "spaceGroteskBold", | ||
| // The way expo-fonts config plugin applies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
praise: thanks for figuring this out! I haven't tried out the new font loading technique so it's good to learn this.
|
Hear me out @coolsoftwaretyler / @frankcalise, I feel like the Activity / Suspense boundary mentioned above feels like it doesn't include preloading other things like auth, i18n, navigation state, etc.. It sort of doesn't seem as fluid to me with the other things we are trying to check before displaying the splash screen. What if we did something like this and introduced a new hook that handled this for us and we just call it with some notes? |
|
@ChristopherGabba - I think that's a good point. It's simpler and fits in with the rest of the hook-based loading, IMO. Do you want to push up a commit with that so we can evaluate? |
0920435 to
5bbf8dd
Compare
|
Okay @coolsoftwaretyler / @jamonholmgren, I think I found a clean approach that allows you to only have to handle the fonts in one place for ios, android, and web. Basically:
By moving the expo-fonts plugin to I did build this on ios, android, and web and the fonts were present on all of them, but you may double check me again. One last comment is that is that I placed the new |
|
I'll take another look soon. Thanks @ChristopherGabba! |
coolsoftwaretyler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ChristopherGabba - this is looking great. I had a couple small suggestions I'd like to see before merge .
boilerplate/app/theme/typography.ts
Outdated
| SpaceGrotesk_700Bold: require("../../assets/fonts/SpaceGrotesk-700Bold.ttf"), | ||
| } | ||
|
|
||
| export const customFontsToLoad = FONT_FILES as Record<string, FontSource> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I prefer to avoid type assertions. Looksl ike we can just provide FontSource as one of the generic arguments to Record. This patch passes yarn compile for me in an ignited app:
diff --git a/app/theme/typography.ts b/app/theme/typography.ts
index 65f7aff..bd437c6 100644
--- a/app/theme/typography.ts
+++ b/app/theme/typography.ts
@@ -4,7 +4,7 @@
import { Platform } from "react-native"
import { FontSource, useFonts } from "expo-font"
-export const FONT_FILES: Record<string, string> = {
+export const FONT_FILES: Record<string, FontSource> = {
SpaceGrotesk_300Light: require("../../assets/fonts/SpaceGrotesk-300Light.ttf"),
SpaceGrotesk_400Regular: require("../../assets/fonts/SpaceGrotesk-400Regular.ttf"),
SpaceGrotesk_500Medium: require("../../assets/fonts/SpaceGrotesk-500Medium.ttf"),
@@ -12,7 +12,7 @@ export const FONT_FILES: Record<string, string> = {
SpaceGrotesk_700Bold: require("../../assets/fonts/SpaceGrotesk-700Bold.ttf"),
}
-export const customFontsToLoad = FONT_FILES as Record<string, FontSource>
+export const customFontsToLoad = FONT_FILES
/**
* On iOS and Android, the fonts are embedded as part of the app binary
boilerplate/app/theme/typography.ts
Outdated
| */ | ||
| export const useCustomFonts = (): [boolean, Error | null] => { | ||
| if (Platform.OS === "web") { | ||
| // eslint-disable-next-line react-hooks/rules-of-hooks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: this is technically safe since Platform.OS is never really going to change at runtime and trigger the problem this rule prevents, but I'd still prefer to comply to the rules of hooks. I think we can get around it by always calling useFonts, and then conditionally returning different values. Like this:
diff --git a/app/theme/typography.ts b/app/theme/typography.ts
index 65f7aff..a78dbb1 100644
--- a/app/theme/typography.ts
+++ b/app/theme/typography.ts
@@ -24,9 +24,10 @@ export const customFontsToLoad = FONT_FILES as Record<string, FontSource>
* For more info: https://docs.expo.dev/versions/latest/sdk/font/
*/
export const useCustomFonts = (): [boolean, Error | null] => {
+ const [areFontsLoaded, fontLoadError] = useFonts(customFontsToLoad)
+
if (Platform.OS === "web") {
- // eslint-disable-next-line react-hooks/rules-of-hooks
- return useFonts(customFontsToLoad)
+ return [areFontsLoaded, fontLoadError]
}
// On native, fonts are precompiled and ready
package.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: let's undo this change before merge, please.
|
Okay @coolsoftwaretyler, great comments. I actually figured out a way to rename the files and make all the platforms line up on the file imports! Seems to work great! Check it out and let me know what else you think is needed. |
|
Looks great, @ChristopherGabba - will you please rebase and fix the conflict? I think we're good after that. |
|
Did I do it? haha never rebased before. |
|
Looks like you merged instead of rebasing (here's the difference). That's OK though, we don't have a super active main branch, and if you resolved conflicts through the GitHub UI, that's what they usually do by default anyway. |
coolsoftwaretyler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and works. Thanks for all the iteration, @ChristopherGabba. I'm gonna post about this internally and get one more review before we merge.
|
Awesome! I've been experimenting with different fonts on import and there's definitely some insistency between different fonts. There may be some fonts where we need to use Platform.select still. Montserrat is one of those where on iOS, the weight is not in the font name. Can you run an iOS build on your end and make sure it's correctly working? |
|
Hey @ChristopherGabba - can you say more about the inconsistency you're seeing? Maybe post a couple screenshots? I don't think we use Montserrat here. |
|
@coolsoftwaretyler No, I'm just using Monserrat in my personal app, but I have to differentiate between the iOS and android platforms for that particular font. Note how I'm importing them all with numbers, but the way to reference them still requires me to differentiate. For some reason, SpaceGrotesk with numbers just works for both without |
|
Whoa I just figured it out, do not merge this. By following the rules of hooks, I masked the error: export const useCustomFonts = (): [boolean, Error | null] => {
const [areFontsLoaded, fontError] = useFonts(customFontsToLoad)
if (Platform.OS === "web") {
return [areFontsLoaded, fontError]
}
// On native, fonts are precompiled and ready
return [true, null]
}This was dynamically loading the font every time regardless and masking the fact that my config plugin wasn't working. I'll fix it, give me a minute. |
|
There we go, now its consistent with mine, you do need to use Platform.select on ios I was wrong, when I fixed the hook per your recommendation, it was dynamically loading the fonts all over again every time. |
|
Ah that makes sense! I'll take another look sometime this week or next. Good catch and thanks for the fix. |
frankcalise
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! I left some feedback - couple of things to clean up and one or two items to dig a little deeper on
|
Just want to applaud both of you, you are great engineers! Thanks for the guidance, and good team work! I made the changes suggested by Frank as well, and that lined out the Platforms and removed |
|
The updates look good! Thanks for doing this. I'll try and get to pulling down the branch and testing the changes later today. |
|
Sounds good! Hopefully it all works! |
Description
I came across this X.com thread: https://x.com/aleqsio/status/1968273285239734405 and I read through the expo font docs: https://docs.expo.dev/versions/latest/sdk/font/
The reason: speed up initial app checks by removing the
fontLoadcheck on iOS and Android.Based off everything I've read, there is no way to do this on web so I tried to account for that on the PR. Maybe someone from expo can shed some light on this, but the docs don't mention web on the config plugin.
useFontsand embed them as part of the App Binary #2995Screenshots
It appears to be working on all three platforms.
Android:

iOS:
Web:
Checklist