-
Notifications
You must be signed in to change notification settings - Fork 18
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
Make font configurable, improve asset handling and use basic frontend bundle splitting #1167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LukasKalbertodt
force-pushed
the
better-assets
branch
from
May 15, 2024 13:50
726499d
to
341f39c
Compare
owi92
approved these changes
May 15, 2024
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.
Very nice! Looking forward to having less overhead when adding new things to our config. Extra stars for the docs ;)
This finally (!!) revamps our asset handling. We use reinda, which does a lot for us, but it had some limitations before. Specifically, it could not embed files by wildcard pattern and files could not be added purely at runtime. This made frontend code splitting and including custom fonts impossible. To improve this, reinda was almost completely rewritten. The most important changes from reinda and this commit: - No more reinda templates: it has been replaced by the "modifier" API in reinda, which gives Tobira more control. This gets rid of the million manual variables in our index.html. - The `embed!` macro only mentions files that are actually embedded into the executable. Logos for example are added at runtime, without a weird "path_override". - The compression of embedded assets is now done with brotli, which should result in a smaller binary. - The "logo fallback logic" is now performed in the frontend. This is more convenient for us and avoids duplicating the logo contents in memory. - Fonts and Paella icons are embedded by wildcard now, so we don't have to list every darn file anymore. I am slightly uneasy as this means in the future, we might accidentally include unneeded files there. I don't think reinda's API is "perfect" now. My main complain is the duplication of paths/filenames. But we want to ship that at some point and I think the API works well enough. The duplication here is unlikely to result in bad bugs that are only noticed late. NOTE: This does not restore full previous functionality yet! Two things are missing: - The font paths in `fonts.css` are not adjusted and broken. - The frontend code map is not included yet. Both of these will be fixed in the next commits.
Admins can modify the `font-family` used by Tobira, they can specify font files that Tobira will serve, and they can specify extra CSS code that can add more `@font-face` declarations. This should be a simple and flexible system to configure custom fonts.
This is the first step to using code splitting to reduce the loaded JS size. This commit lays the ground work but only splits the bundle into two files. But that's already a big win as it cuts the size of JS one has to download to render the main page roughly in half. Even if there is a player on the main page, the page already renders once the main bundle is loaded, and the player shows a placeholder spinner. In the future, we can create more bundles, e.g. moving everything "manage" related to another bundle or even just letting webpack decide.
This is more straight forward. Before this commit, the setup was broken as the "find main bundle" code in `assets.rs` was incorrect. It only iterated over files embedded at compile time, which might be missing the bundle. But more: if the frontend was changed, the hash and thus filename of the main bundle changed as well, and the backend replacement would be incorrect. Letting webpack handle this part seems simpler. This commit also deduplicates some strings in `assets.rs`.
LukasKalbertodt
force-pushed
the
better-assets
branch
from
May 15, 2024 14:49
341f39c
to
a962cab
Compare
LukasKalbertodt
force-pushed
the
better-assets
branch
from
May 15, 2024 14:57
a962cab
to
cd58b76
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #287
Fixes #362
First steps towards #257
Historical background pretty irrelevant for users of Tobira
This was on my radar for years already, and we always talked about making fonts configurable and splitting the frontend. Both were blocked by us using
reinda
as a library to manage assets, which had some limitations (mainly: not being able to include wildcards). I tried tackling this a couple of time, but never succeeded. This time I wanted to finally make it work, since we needed configurable fonts.And boy, it was a lot more complex than I thought. This seems very simple on the surface but the complexity comes from the two different modes: in release mode we want to embed most assets into the executable, but in debug mode we want to load everything dynamically for faster development speed. Neither of these things are really up for debate. Combine that with the need to have hashes in file names, which means that references to assets need to be fixed, and you have a very complex system. I'm convinced this would have been at most marginally easier when doing that all inside Tobira, instead of having that logic live in an external library. So yeah, as far as I see this was just a hard task. I am probably only writing this here so that I don't feel so bad for taking such a long time 😭
Anyway, I am happy with the result, both the library and how it works in Tobira. It also gets rid of this terrible frontend config mess we had before. Now it's mostly just one
json!{}
call and easy to read.The user/admin-facing change in this PR is configurable font. Admins can specify a font-family, custom CSS (with
@font-face
declarations) and a list of font files in the config. The font files are served as static files, the CSS and font-family is included. This should be plenty flexible! The default Open Sans font is still always included, I don't think that hurts.See the changes in
config.toml
for more information.The other change, which is only indirectly user-facing is that I started code splitting. This was just to verify it's possible with the new
reinda
, it's absolutely not perfect yet. I just split the biggest dependency in our bundle (Paella) into its own file. This reduces the JS size required to render the main site considerably (3.0MB -> 1.3MB).Since the new reinda also uses
brotli
instead offlate2
, the Tobira binary size might have shrunk a bit. EDIT: nope, not considerably: 34.8MB -> 34.6MB. For the record: size of embedded assets is 2.4MiB in total (13.9MiB when uncompressed).Finally:
reinda
is not yet released. I wanted to wait for this PR to get reviewed before doing that. Once I get a thumbs up, I will release reinda, adjustCargo.toml
and then this PR can be merged.(PR can be reviewed commit by commit)