wevm / frog

Framework for Farcaster Frames 🐸
https://frog.fm
Other
423 stars 97 forks source link

External fonts are rendered inconsistently #303

Closed dalechyn closed 5 months ago

dalechyn commented 6 months ago

Describe the bug

ref: https://github.com/wevm/frog/pull/215#issuecomment-2094011166

Looks like fonts are not applied when fetched inside frame handler.

Link to Minimal Reproducible Example

https://github.com/dalechyn/frog-font-inconsistency

Steps To Reproduce

No response

Frog Version

0.9.2

TypeScript Version

5.4.5

Check existing issues

Anything else?

No response

tmm commented 5 months ago

Can we close this now?

dalechyn commented 5 months ago

Can we close this now?

No, still reproducible. taking a look at it rn.

dalechyn commented 5 months ago

The core issue arises from the fact that ArrayBuffer is not serializable by nature.

Since fonts is an accepted property in frame handler's imageOptions, there's no way local fonts can be supported there as we cannot serialize the ArrayBuffer.

See: https://github.com/dalechyn/frog-font-inconsistency/blob/379511685f1421ae24c55203097152c052b78c8a/api/index.tsx#L63-L71.

Moreover, trying to pass non-local fonts would not also work because of this: https://github.com/wevm/frog/blob/a98f567e878611609911dcca9abb28f000d1ea4a/src/frog-base.tsx#L580-L581.

considering the mentioned above, I don't think it's worth making it work since user can define fonts in Frog constructor which would not have to be needed to be serialized, and if one needs multiple fonts he better would have to import his fonts there.

scottrepreneur commented 5 months ago

Thanks for digging into this guys. Can you also clarify in the docs where this overlaps with using a local/custom font within Frog UI here? Seems the same implementation works over there.

https://frog.fm/ui/createSystem#fonts

madroneropaulo commented 5 months ago

I can confirm that declaring fonts in the Frog constructor works. To me that's enough since fonts can be imported there and used everywhere. I can't think of an scenario where that wouldn't be enough. I'd only suggest making it VERY clear in the docs and that's it. Thanks!