这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@ChristopherGabba
Copy link

@ChristopherGabba ChristopherGabba commented Oct 6, 2025

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 fontLoad check 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.

Screenshots

It appears to be working on all three platforms.

Android:
Screenshot 2025-10-06 at 2 04 28 PM

iOS:

IMG_0293

Web:

Screenshot 2025-10-06 at 2 47 02 PM

Checklist

  • I have manually tested this, including by generating a new app locally (see docs).

@ChristopherGabba
Copy link
Author

Also I apologize, I somehow back tracked a release ci PR.. my bad. Idk how to undo that part.

Copy link
Contributor

@coolsoftwaretyler coolsoftwaretyler left a 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?

Comment on lines 72 to 76
// 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)
Copy link
Contributor

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:

https://github.com/EvanBacon/expo-router-forms-components/blob/5aa311c507863faff99ad3853ac7607f487e7c71/src/app/_layout.tsx#L3

I'd prefer that implementation, myself.

Copy link
Author

@ChristopherGabba ChristopherGabba Oct 13, 2025

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?

Comment on lines 38 to 49
[
"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"
]
}
],
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"version": "11.2.0",

suggestion: no need for this, I think CI will do it for us.

Copy link
Contributor

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.

medium: "spaceGroteskMedium",
semiBold: "spaceGroteskSemiBold",
bold: "spaceGroteskBold",
// The way expo-fonts config plugin applies
Copy link
Contributor

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.

@ChristopherGabba
Copy link
Author

ChristopherGabba commented Oct 13, 2025

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?

Screenshot 2025-10-13 at 11 41 19 AM

@coolsoftwaretyler
Copy link
Contributor

@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?

@ChristopherGabba
Copy link
Author

ChristopherGabba commented Oct 15, 2025

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:

  1. Drag the font files into assets/fonts.
  2. Update the imports in the typography.ts.
  3. Run npx expo prebuild

By moving the expo-fonts plugin to app.config.ts, we can dynamically bring the files in from typography.ts and we don't have to duplicate it in multiple places. Let me know your thoughts.

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 useCustomFonts hook in the typography. Personally I feel like this makes sense to have it in once place, but it could be in its own useCustomFonts hook file.

@coolsoftwaretyler
Copy link
Contributor

I'll take another look soon. Thanks @ChristopherGabba!

Copy link
Contributor

@coolsoftwaretyler coolsoftwaretyler left a 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 .

SpaceGrotesk_700Bold: require("../../assets/fonts/SpaceGrotesk-700Bold.ttf"),
}

export const customFontsToLoad = FONT_FILES as Record<string, FontSource>
Copy link
Contributor

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

*/
export const useCustomFonts = (): [boolean, Error | null] => {
if (Platform.OS === "web") {
// eslint-disable-next-line react-hooks/rules-of-hooks
Copy link
Contributor

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
Copy link
Contributor

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.

@ChristopherGabba
Copy link
Author

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.

@coolsoftwaretyler
Copy link
Contributor

Looks great, @ChristopherGabba - will you please rebase and fix the conflict? I think we're good after that.

@ChristopherGabba
Copy link
Author

Did I do it? haha never rebased before.

@coolsoftwaretyler
Copy link
Contributor

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.

Copy link
Contributor

@coolsoftwaretyler coolsoftwaretyler left a 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.

@ChristopherGabba
Copy link
Author

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?

@coolsoftwaretyler
Copy link
Contributor

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.

@ChristopherGabba
Copy link
Author

ChristopherGabba commented Oct 16, 2025

@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.

Screenshot 2025-10-16 at 10 29 01 AM

Note how I'm importing them all with numbers, but the way to reference them still requires me to differentiate.

Screenshot 2025-10-16 at 10 29 32 AM

For some reason, SpaceGrotesk with numbers just works for both without Platform.select. I don't fully understand why because when I go to the info.plist for my app, they show up as numbers. But they literally do not render on iOS unless I use the Platform.select..

Screenshot 2025-10-16 at 10 31 11 AM

@ChristopherGabba
Copy link
Author

ChristopherGabba commented Oct 16, 2025

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.

@ChristopherGabba
Copy link
Author

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.

@coolsoftwaretyler
Copy link
Contributor

Ah that makes sense! I'll take another look sometime this week or next. Good catch and thanks for the fix.

Copy link
Contributor

@frankcalise frankcalise left a 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

@ChristopherGabba
Copy link
Author

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 Platform.select. I also added a console.warn in the config plugin function just to warn the developer if for some reason the fonts are detected in the folder. I also removed the .ttf, didn't realize there were other types!

@frankcalise
Copy link
Contributor

The updates look good! Thanks for doing this.

I'll try and get to pulling down the branch and testing the changes later today.

@ChristopherGabba
Copy link
Author

Sounds good! Hopefully it all works!

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.

3 participants