feat(fonts): warn if local font is in public dir

[florian-lefebvre]

Changes

• RFC feedback
• People migrating to the fonts API with local fonts tend to leave their fonts in public, but this is not recommended as we copy fonts so files are effectively duplicated in the build output

Testing

Manual

Docs

withastro/docs#11511 + changeset

[changeset-bot]

🦋 Changeset detected

Latest commit: c06f9bd

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codspeed-hq]

CodSpeed Performance Report

Merging #13678 will not alter performance

_{Comparing feat/fonts-warn-public-dir (c06f9bd) with main (42388b2)}

Summary

✅ 6 untouched benchmarks

[ArmandPhilippot]

Docs review (but you probably want the opinion of a native speaker 😄 ): the warning looks fine to me! I only left a nit regarding the changeset. So, LGTM.

[sarah11918]

So, I left a comment about the warning itself, but I have a question about having this warning in the first place.

So, with this warning, someone with a font file in public/ will see this EVERY time? Why would someone choose to keep it in public, and if there is a good reason and they're intentionally doing this, they probably don't want to see a warning every time? Just making sure that this won't affect someone this way.

I can see in the docs PR we're suggesting not to do this, so presumably it's OK if the warning suggestions people should move their file. But just want to make sure there's not a good reason for not doing so, and whether people are signing up for this error when maybe just having docs is enough?

[JusticeMatthew]

So, with this warning, someone with a font file in public/ will see this EVERY time? Why would someone choose to keep it in public, and if there is a good reason and they're intentionally doing this, they probably don't want to see a warning every time? Just making sure that this won't affect someone this way.

I can't think of a use case where keeping fonts in public would be better, after having spent many hours optimizing fonts in my own projects

This warning does seem like a good thing to have for the very specific users that Florian mentions who have perhaps forgotten to move/delete the fonts from public after upgrading to the new API

[florian-lefebvre]

So, with this warning, someone with a font file in public/ will see this EVERY time?

Only when using the experimental fonts API

Why would someone choose to keep it in public

I don't think there's any good reason to do so when using this feature

[ematipico]

As for now, the warning is fine because it's an experimental feature, however I don't think we should keep it once it's out of experimental.

We don't warn users if they have their images in public/, so we shouldn't for fonts too.

It's up to the users to decide what to do, as long as docs warn them.

[florian-lefebvre]

Good point, i'll add a todo comment

[sarah11918]

I agree with Ema that when stable, this doesn't feel like a necessary warning but instead something to document. But the message as written is fine by me!