Skip to content
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

Dont Work in production #5

Open
profedgar opened this issue Mar 14, 2020 · 16 comments
Open

Dont Work in production #5

profedgar opened this issue Mar 14, 2020 · 16 comments

Comments

@profedgar
Copy link

i have on issue in generated apk, external fonts dont load or download

@cawfree
Copy link
Owner

cawfree commented May 11, 2020

Hey there, sorry for the delay getting back to you. Looks like I have some trouble getting notifications. What is the error you're seeing?

@Munoon
Copy link

Munoon commented May 26, 2020

It looks like I have the same thing.
On simulator everything works fine, but then I tried to install it on my device, using react-native run-ios —device "iPhone Munoon". Here is my exception:
image

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

Have you tried upgrading to the latest version (1.2.0-beta.1)? In previous versions, it was possible to access fonts before they'd been downloaded and installed. The reason it works on simulator is likely because of timing. Your actual device may take slightly longer to download...

I've got this working on a production iOS + Android application, so hopefully it'll give you some confidence that there shouldn't be any native changes required.

@Munoon
Copy link

Munoon commented May 26, 2020

Yeah, this demo was launched on ^1.2.0-beta.1" version.
"react-native-custom-fonts": "^1.2.0-beta.1" setting in package.json.

By the way, after I tried to launch it on my device, it was broken on simulator too. No changes was made... I, even, tried to launch old version.

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

Ah okay, thanks a lot for letting me know.

Do you have a minimum reproducible example? Otherwise, are you able to provide the uri, fontFamily and fontWeight you're using? I'll do some experimenting and see what I can come up with.

@Munoon
Copy link

Munoon commented May 26, 2020

Sure, here it is:

'Lacquer': {
    uri: "http://localhost:8080/resources/fonts/lacquer/lacquer-v1-latin-regular.ttf",
    fontFamily: "Lacquer",
    fontWeight: "400",
    fontStyle: "normal",
    color: "white"
}

Yes, I'm sure, that server is running and font downloading properly.

Font can be downloaded here: https://grandschool.net.ua/resources/fonts/lacquer/lacquer-v1-latin-regular.ttf

By the way, I found strange thing. It works fine, after I started debug session. Even, if I have ended it, it is still working. But if I restart app - same problem.

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

Hmm, sounds like the fonts are taking a while to download and register natively. I'll have a play around and I'll see if I can come up with anything.

Thanks a lot for your help!

@Munoon
Copy link

Munoon commented May 26, 2020

Just discovered that you are right. If you close the error - font is loaded correctly!

@Munoon
Copy link

Munoon commented May 26, 2020

Fix this issue by creating state fontLoading and use font only if fontLoading === false. Set value false in onDownloadDidEnd handler, but I need to check if this value isn't already false, because handler may be called many times. This is not very convenient.

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

Agreed, it's not expected as a consumer of the library for you to have to do this. Unfortunately, I ran this font in the example code and it worked first try. Are you by any chance specifying the fontFamily anywhere manually, other than inside the fontFaces prop?

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

So one thing I'm thinking of is that, on iOS, if the request to load the fonts globally fails, it swallows the error on the native runtime.

This causes the JS runtime to happily assign the font faces, even though they've not been properly downloaded. So, if an error was being thrown, we're not seeing it and the app is falling into a soft error. I believe the reason I originally did this was to exploit the native error window as feedback for the failure.

But, this is just a hunch. I think we're seeing that the fonts are downloaded as expected, so I'm wondering if there's something else at play.

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

The fact that you can just useState as a workaround just screams of a timing issue. It just bothers me that there's nothing really special about the callback methods; just your state update will be a render frame afterwards.

... So in that case...

What I can do is requestAnimationFrame after the fonts have been downloaded before updating the views. That's if the crash truly has disappeared after applying your workaround?

@Munoon
Copy link

Munoon commented May 26, 2020

Yes, since I already use style preferences in my components, I have to create an array of styles, where I add the result of calling the useCustomFont function.

You can look at code here.

@cawfree
Copy link
Owner

cawfree commented May 26, 2020

Not going to lie, it is terribly exciting to see a library in a project of this scale! 🥳

One thing I'm thinking is that, I don't think you can really guarantee when the application is going to be re-rendered. So whenever you call getFontFace, depending on the progress of the download, you will run into the problem of trying to use a font that doesn't yet exist. The reason it works after your first run is because the fonts have been downloaded and registered with the system previously. First time though, there's no way they exist.

So, in the hooks-based approach, it basically manages access to the fonts in a similar way that you've implemented, just on a granular per-component level.

The CustomFontsProvider doesn't let the allocated style return the appropriate font until it has been downloaded; it reverts a default style and forces re-render only once they're available. In your case, there's no telling when a call to getCustomFont is made, and there's effectively a race condition against the app.

In this regard, it kind of makes sense to use fontLoading state manages visibility of the font, since this is what happens internally.

Additionally, the CustomFontsProvider doesn't do anything smart. So, if your object of fontFaces changes, it's going to attempt a download again, because the object itself has changed. This means every time getCustomFonts() is called, it creates a new object and forces the fonts to re-download. This is probably unlikely given the order of priority of the provider, but it's still likely...

I think the approach you've come up with is best, although I can understand your dissatisfaction. In older versions of RNCustomFonts, I had implementors use a dedicated Text and TextInput module exported from the library, which did workaround this issue, but it massively reduced its scope for reuse in conjunction with custom text fields.

Currently, there's an unavoidable delay between when the fonts have been downloaded and when they're ready to render, and we have to manage that to avoid these errors.

So from this point, it's up to you.

One of the main reasons I made this library is because it is a complete pain managing native font assets, and it makes it very difficult to try out new fonts. If in your solution you don't require the fonts to change at runtime, I think you'd still benefit a lot from being able to rapidly prototype with the fonts + styles which suit your application, and then once you're ready, make the proper native integration.

However, if you do require fonts to change at run time (for example I use this in White Label Applications), you might have to take the hit with the fontLoading state, unless you can think of anything else which I could provide which might make this easier for you?

@waldemirflj
Copy link

Can someone help me.

image

@savaughn
Copy link

Can someone help me.

Looks like you need to npm install the package. check your package.json and make sure that the lib is present.

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

No branches or pull requests

5 participants