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

Static hosting of google fonts #1608

Merged
merged 4 commits into from Sep 18, 2022
Merged

Static hosting of google fonts #1608

merged 4 commits into from Sep 18, 2022

Conversation

cw789
Copy link
Contributor

@cw789 cw789 commented Sep 16, 2022

To be discussed

Close #1607

@josevalim
Copy link
Member

Yes, let's go ahead. GDPR is a strong enough argument to me.

@jonatanklosko
Copy link
Member

jonatanklosko commented Sep 16, 2022

Not sure if it works well with esbuild, but there's a @fontsource/* npm package for every font from google fonts, like @fontsource/merriweather, which simplifies it into a single import.

@cw789
Copy link
Contributor Author

cw789 commented Sep 16, 2022

I think it also worked with esbuild without problems. At least if mix build does nothing different then mix docs will do in projects.

But I will check @fontsource/* packages next week. Possibly it's anyway the better solution.

@jonatanklosko
Copy link
Member

@cw789 to clarify, I meant that I'm not sure if @fontsource/* will work with esbuild, but if it does that would be great :)

@cw789
Copy link
Contributor Author

cw789 commented Sep 17, 2022

Hi @jonatanklosko. Sorry to misunderstood you first. Thanks for clarification.
So @fontsource/* works perfectly fine with esbuild.

But they ship still legacy .woff. In my opinion we wouldn't need them anymore.
What we could do, is to not copy .woff over to dist. In CSS it will still remain.

The additional size in dist we will get (if it matters ;):
@fontsource with .woff2 only: 288 kB
@fontsource + additional .woff: + 298 kB
Manually, only .woff2, only in latin, within the assets directory as I did in the beginning: 115 kB

@josevalim
Copy link
Member

I don’t think we should be Latin only because someone can write documentation in Chinese, right?

But I am fine with getting rid of woff.

@cw789
Copy link
Contributor Author

cw789 commented Sep 17, 2022

I don’t think we should be Latin only because someone can write documentation in Chinese, right?

I also think so.

But I am fine with getting rid of woff.

Done.

From my side it's ready then.

@cw789 cw789 marked this pull request as ready for review September 17, 2022 09:14
@josevalim
Copy link
Member

Thank you! I am thinking we should revert to your original commit and host them ourselves as we don't need woff. Thoughts?

@josevalim
Copy link
Member

Ah, maybe your original commit is latin only?

@jonatanklosko
Copy link
Member

As far as I understand a888a90 got rid of woff, but I don't see it in the final diff for some reason.

@cw789
Copy link
Contributor Author

cw789 commented Sep 18, 2022

We can go with my initial solution (all fonts within our assets directory),
but yes I would need to add the additional character sets (it was more just a proof of concept, with latin only as starting point).

The @fontsource/* solution is also quite fine in my opinion.
To clarify the behavior:

  • it includes the legacy woff in the imported CSS → Lato example
  • esbuild needs to process the legacy woff with its file loader
    Otherwise esbuild will fail, I think there is no loader to just ignore .woff.
    But this is just a minor overhead of the build process.
    // TODO: Remove when @fontsource/* removes legacy .woff
    '.woff': 'file',
  • Our existing HTML formatter does not copy woff from the assets to the final docs/dist directory.
    I added this behavior and just removed it in commit a888a90 again.
    That's why it's not in the final diff.

So my conclusion about the @fontsource/* solution is - it is also really fine.
We would have woff just in the build process and in CSS.
But browsers will anyway prefer woff2 and the ~2 % of browsers just supporting woff
will just fail to load the font and fallback to a local font.

What solution should we prefer?
I'm good with both of them.

@josevalim josevalim merged commit 9092457 into elixir-lang:main Sep 18, 2022
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

@cw789 cw789 deleted the google_fonts branch September 18, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Static hosting of google fonts
3 participants