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
feat: self host api-reference fonts on scalar CDN #1520
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 296bb94 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
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 |
#1589 Bundle Size — 1.92MiB (+0.16%).Warning Bundle contains 4 duplicate packages – View duplicate packages Bundle metrics
|
Current #1589 |
Baseline #1587 |
|
---|---|---|
Initial JS | 1.92MiB (+0.16% ) |
1.91MiB |
Initial CSS | 0B |
0B |
Cache Invalidation | 100% |
100% |
Chunks | 1 |
1 |
Assets | 1 |
1 |
Modules | 1083 |
1083 |
Duplicate Modules | 0 |
0 |
Duplicate Code | 0% |
0% |
Packages | 158 |
158 |
Duplicate Packages | 4 |
4 |
Bundle analysis report Branch tom/host-fonts-on-cdn Project dashboard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Any chance we can make them .css
files? That’s what they are, also I assume the mimetype will be fixed then (currently text/plain, should be text/css).
b97bc28
to
9ad0a49
Compare
9ad0a49
to
243918c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Love how clean the CDN URL is now. Just one thing: Both CSS files still point to Google, for example:
We need to change this to be an URL on our CDN. We don’t want to hit the Google Fonts servers anymore and their URL with the cache hash might change and break in the future.
4f8d759
to
c31bf8d
Compare
Good idea. We will now have our own font css files and host the .woff2 font files on the CDN. This should reduce the number of requests necessary 😄 |
c31bf8d
to
3a1909e
Compare
3a1909e
to
b9b7c4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So clean, so good. Good bye, Google Fonts! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like were only hosting and setting up the regular font weight (400) so bold it just being interpolated by the browser (poorly) off of the regular font.
This branch (left) vs main (right)
I've opened a PR (#1576) against this branch to switch us to the variable width version of Inter (which we should do anyway), the CDN font files will need to be updated as well.
So great you spotted this! 🙌 |
we are missing some fonts here and weights right @cameronrohani ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are missing some weights right @cameronrohani ?
Shouldn't be once we get the variable width font uploaded to the CDN, you'll be able to set any font weight from 100 → 900 (including in between weights like 421 or something), we only use 400, 500 and 600 in our CSS variables but this gives us the option to change in the future or for people themeing to use other font weights. From Google:
|
c2f4fb6
to
d2ffcdd
Compare
6ebe762
to
b697e07
Compare
b697e07
to
2bf079d
Compare
I think this is ready to be merged, needs your final approvement @marclave. 🙏 Let me know if I can further help with that. |
2bf079d
to
1fad47d
Compare
1fad47d
to
296bb94
Compare
Preview deployed to https://24622688-0f22-4e70-bcf3-70a87b4ca3da--scalar-deploy-preview.netlify.app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested! let's go!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait just thinking if we can add a flag to quickly add google fonts 🤔
This PR hosts default api-reference fonts on the scalar CDN. This is currently a drop-in replacement but further optimizations can be made by removing unused alphabets and selectively fetching based on the selected language.