Constants should always be only assigned once. The logical OR assignment
of a constant is a relic of the past before we used zeitwerk for
autoloading and had bugs where a file could be loaded twice resulting in
constant redefinition warnings.
This commit moves the generation of category background CSS from the
server side to the client side. This simplifies the server side code
because it does not need to check which categories are visible to the
current user.
Why this change?
The `can survive cache miss` test in `spec/requests/stylesheets_controller_spec.rb`
was failing because the file was not found on disk for the cache to be
regenerated. This is because a test in
`spec/lib/stylesheet/manager_spec.rb` was removing the entire
`tmp/stylesheet-cache` directory which is incorrect because the folder
in the test environment further segretates the stylesheet caches based
on the process of the test.
What does this change do?
1. Introduce `Stylesheet::Manager.rm_cache_folder` method for the test
environment to properly clean up the cache folder.
2. Make `Stylesheet::Manager::CACHE_PATH` a private constant since the
cache path should be obtained from the `Stylesheet::Manager.cache_fullpath` method.
Prior to this commit, we didn't have RTL versions of our admin and plugins CSS bundles and we always served LTR versions of those bundles even when users used an RTL locale, causing admin and plugins UI elements to never look as good as when an LTR locale was used. Example of UI issues prior to this commit were: missing margins, borders on the wrong side and buttons too close to each other etc.
This commit creates an RTL version for the admin CSS bundle as well as RTL bundles for all the installed plugins and serves those RTL bundles to users/sites who use RTL locales.
Previously the stylesheet cachebusting hash was based on the maximum mtime of files. This works well in development and during in-container updates (e.g. via docker_manager). However, when a fresh docker image is created for each deploy, the file mtimes will change even if the contents has not.
This commit changes the production logic to calculate the cachebuster from the filenames and contents of the relevant assets. This should be consistent across deploys, thereby improving cache hits and improving page load times.
* Revert "Revert "FEATURE: Preload resources via link header (#18475)" (#18511)"
This reverts commit 95a57f7e0c.
* put behind feature flag
* env -> global setting
* declare global setting
* forgot one spot
Experiment moving from preload tags in the document head to preload information the the response headers.
While this is a minor improvement in most browsers (headers are parsed before the response body), this allows smart proxies like Cloudflare to "learn" from those headers and build HTTP 103 Early Hints for subsequent requests to the same URI, which will allow the user agent to download and parse our JS/CSS while we are waiting for the server to generate and stream the HTML response.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
This commit adds preload links for core/plugin/theme CSS stylesheets in the head.
Preload links are non-blocking and run in parallel. This means that they should have already been downloaded by the time we use the actual stylesheets (in the <body> tag).
Google is currently complaining about this here and this PR will address that warning.
This commit will also fix an issue in the splash screen where it sometimes doesn't respect the theme colors - causing a slightly jarring experience on dark themes.
Note that I opted not to add new specs because the underlying work required already has a lot of coverage. The new methods only change the output HTML so we can chuck that in the document <head>
This change also means that we can make all the stylesheets non-render blocking, but that will follow in a separate commit.
Without this parameter, requests for sourcemaps on shared-CDN multisites will not be routed to the correct database, resulting in a 404.
The stylesheet content now depends on the site hostname, so the hostname has been added to the digest.
* File.exists? is deprecated and removed in Ruby 3.2 in favor of
File.exist?
* Dir.exists? is deprecated and removed in Ruby 3.2 in favor of
Dir.exist?
If a theme name contained a double-quote, this problem could lead to invalid/unexpected HTML in the `<head>`
Note that this is not considered a security issue because themes can only be installed/named by administrators, and themes/administrators already have the ability to run arbitrary javascript.
The cache_fullpath for the Stylesheet::Manager was the same for
every test runner in a parallel test environment, so when other
specs or other places e.g. the stylesheets_controller_spec ran
rm -rf Stylesheet::Manager.cache_fullpath this caused errors
for other specs running that went through the
Stylesheet::Manager::Builder#compile path, causing the error
```
Errno::ENOENT:
No such file or directory @ rb_sysopen
```
Also fixed the stylesheet_controller which was interpolating Rails.root + CACHE_PATH
itself instead of just using Stylesheet::Manager.cache_fullpath
Take 2 of https://github.com/discourse/discourse/pull/13466.
Fixes a few issues with the original PR:
- color definition stylesheet target now includes the theme id, to avoid themes set to use the default color scheme loading the same stylesheet
- changes the internal cache key for color definition stylesheet to reset the pre-existing cache
Before this change, calling `StyleSheet::Manager.stylesheet_details`
for the first time resulted in multiple queries to the database. This is
because the code was modelled in a way where each `Theme` was loaded
from the database one at a time.
This PR restructures the code such that it allows us to load all the
theme records in a single query. It also allows us to eager load the
required associations upfront. In order to achieve this, I removed the
support of loading multiple themes per request. It was initially added
to support user selectable theme components but the feature was never
completed and abandoned because it wasn't a feature that we thought was
worth building.
When `Theme#all_theme_variables` returns an empty array, we were running
a pointless query in `StyleSheet::Manager#uploads_digest`.
`SELECT "sha1" FROM "theme_fields" INNER JOIN "uploads" ON
"uploads"."id" = "theme_fields"."upload_id" WHERE 1=0`
Setting a key/value pair in DistributedCache involves waiting on the
write to Redis to finish. In most cases, we don't need to wait on the
setting of the cache to finish. We just need to take our return value
and move on.
The query is being executed each time we try and generate the link path
for a stylesheet within the duration of a reqeust. Categories are not
updated that often so repeating this query multiple times a request is
wasteful.
At the time of this commit, there is a `publish_discourse_stylesheet`
ActiveRecord callback on the `Category` model which clears the cache of
`Stylesheet::Manager` each time a category is saved.
Previously, we only precompiled the CSS for parent themes but not for
the child themes. As a result, the CSS for child themes were being
compiled during the first request which made the respond time high for
that request.
Plugins always store their stylesheets under `/assets/stylesheets`, so we can make the glob pattern much more specific. In my local development environment, this increases the speed of `Stylesheet::Manager.max_file_mtime` from ~65ms to ~3ms (20x faster). This significantly improves stylesheet regeneration time, and the responsiveness of the theme admin UI.
Note that this will have negligible effect in production, because in production the value of `max_file_mtime` is aggressively cached.
When building the `scss_load_paths`, we were creating a full export of the theme (including uploads), and not cleaning it up. With many uploads, this can be extremely slow (because it downloads every upload from S3), and the lack of cleanup could cause a disk to fill up over time.
This commit updates the ZipExporter to provide a `with_export_dir` API, which takes care of cleanup. It also adds a kwarg which allows exporting only extra_scss fields. This should make things much faster for themes with many uploads.
This switches to outputting a separate file for each theme component CSS
asset. We have separate CSS plugin files, separate JS files
(for plugins/themes/components), it makes sense to do the same for
component CSS assets.
Benefits:
- easier debugging
- fixes a regression with theme component sourcemaps
- changes to theme components are updated individually
With HTTP/2, there is also no performance downside to having additional
files in the initial request.
This switches to outputting a separate file for each theme component CSS
asset. We have separate CSS plugin files, separate JS files
(for plugins/themes/components), it makes sense to do the same for
component CSS assets.
Benefits:
- easier debugging
- fixes a regression with theme component sourcemaps
- changes to theme components are updated individually
With HTTP/2, there is also no performance downside to having additional
files in the initial request.
Allows site administrators to pick different fonts for headings in the wizard and in their site settings. Also correctly displays the header logos in wizard previews.
Alan Guo Xiang Tan
Bianca Nenciu
Daniel Waterworth
Jarek Radosz
Osama Sayegh
David Taylor
Rafael dos Santos Silva
Joe
Loïc Guitaut
Peter Zhu
Penar Musaraj
Martin Brennan