Rework assets loading to fix splitting CSS chunks

crates/spa/src/vite.rs

@@ -1,4 +1,4 @@
-// Copyright 2023 The Matrix.org Foundation C.I.C.
+// Copyright 2023, 2024 The Matrix.org Foundation C.I.C.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -40,6 +40,7 @@ pub struct ManifestEntry {
 
     imports: Option<Vec<Utf8PathBuf>>,
 
+    #[allow(dead_code)]
     dynamic_imports: Option<Vec<Utf8PathBuf>>,
 
     integrity: Option<String>,
@@ -191,35 +192,24 @@ impl<'a> Asset<'a> {
 impl Manifest {
     /// Find all assets which should be loaded for a given entrypoint
     ///
-    /// # Errors
-    ///
-    /// Returns an error if the entrypoint is invalid for this manifest
-    pub fn assets_for<'a>(
-        &'a self,
-        entrypoint: &'a Utf8Path,
-    ) -> Result<BTreeSet<Asset<'a>>, InvalidManifest<'a>> {
-        let entry = self.lookup_by_name(entrypoint)?;
-        let main_asset = Asset::new(entry)?;
-        entry
-            .css
-            .iter()
-            .flatten()
-            .map(|name| self.lookup_by_file(name).and_then(Asset::new))
-            .chain(std::iter::once(Ok(main_asset)))
-            .collect()
-    }
-
-    /// Find all assets which should be preloaded for a given entrypoint
+    /// Returns the main asset and all the assets it imports
     ///
     /// # Errors
     ///
     /// Returns an error if the entrypoint is invalid for this manifest
-    pub fn preload_for<'a>(
+    pub fn find_assets<'a>(
         &'a self,
         entrypoint: &'a Utf8Path,
-    ) -> Result<BTreeSet<Asset<'a>>, InvalidManifest<'a>> {
+    ) -> Result<(Asset<'a>, BTreeSet<Asset<'a>>), InvalidManifest<'a>> {
         let entry = self.lookup_by_name(entrypoint)?;
-        self.find_preload(entry)
+        let mut entries = BTreeSet::new();
+        let main_asset = self.find_imported_chunks(entry, &mut entries)?;
+
+        // Remove the main asset from the set of imported entries. We had it mainly to
+        // deduplicate the list of assets, but we don't want to include it twice
+        entries.remove(&main_asset);
+
+        Ok((main_asset, entries))
     }
 
     /// Lookup an entry in the manifest by its original name
@@ -243,41 +233,38 @@ impl Manifest {
             .ok_or(InvalidManifest::CantFindAssetByFile { file })
     }
 
-    /// Recursively find all the assets that should be preloaded
-    fn find_preload<'a>(
-        &'a self,
-        entry: &'a ManifestEntry,
-    ) -> Result<BTreeSet<Asset<'a>>, InvalidManifest<'a>> {
-        let mut entries = BTreeSet::new();
-        self.find_preload_rec(entry, &mut entries)?;
-        Ok(entries)
-    }
-
-    fn find_preload_rec<'a>(
+    fn find_imported_chunks<'a>(
         &'a self,
         current_entry: &'a ManifestEntry,
         entries: &mut BTreeSet<Asset<'a>>,
-    ) -> Result<(), InvalidManifest<'a>> {
+    ) -> Result<Asset, InvalidManifest<'a>> {
         let asset = Asset::new(current_entry)?;
         let inserted = entries.insert(asset);
 
         // If we inserted the entry, we need to find its dependencies
         if inserted {
-            let css = current_entry.css.iter().flatten();
-            let assets = current_entry.assets.iter().flatten();
-            for name in css.chain(assets) {
-                let entry = self.lookup_by_file(name)?;
-                self.find_preload_rec(entry, entries)?;
+            if let Some(css) = &current_entry.css {
+                for file in css {
+                    let entry = self.lookup_by_file(file)?;
+                    self.find_imported_chunks(entry, entries)?;
+                }
+            }
+
+            if let Some(assets) = &current_entry.assets {
+                for file in assets {
+                    let entry = self.lookup_by_file(file)?;
+                    self.find_imported_chunks(entry, entries)?;
+                }
             }
 
-            let dynamic_imports = current_entry.dynamic_imports.iter().flatten();
-            let imports = current_entry.imports.iter().flatten();
-            for import in dynamic_imports.chain(imports) {
-                let entry = self.lookup_by_name(import)?;
-                self.find_preload_rec(entry, entries)?;
+            if let Some(imports) = &current_entry.imports {
+                for import in imports {
+                    let entry = self.lookup_by_name(import)?;
+                    self.find_imported_chunks(entry, entries)?;
+                }
             }
         }
 
-        Ok(())
+        Ok(asset)
     }
 }

crates/templates/src/functions.rs

@@ -18,7 +18,7 @@
 //! Additional functions, tests and filters used in templates
 
 use std::{
-    collections::{BTreeSet, HashMap},
+    collections::HashMap,
     fmt::Formatter,
     str::FromStr,
     sync::{atomic::AtomicUsize, Arc},
@@ -441,41 +441,26 @@ impl std::fmt::Display for IncludeAsset {
 
 impl Object for IncludeAsset {
     fn call(self: &Arc<Self>, _state: &State, args: &[Value]) -> Result<Value, Error> {
-        let (path, kwargs): (&str, Kwargs) = from_args(args)?;
-
-        let preload = kwargs.get("preload").unwrap_or(false);
-        kwargs.assert_all_used()?;
+        let (path,): (&str,) = from_args(args)?;
 
         let path: &Utf8Path = path.into();
 
-        let assets = self.vite_manifest.assets_for(path).map_err(|_e| {
+        let (main, imported) = self.vite_manifest.find_assets(path).map_err(|_e| {
             Error::new(
                 ErrorKind::InvalidOperation,
                 "Invalid assets manifest while calling function `include_asset`",
             )
         })?;
 
-        let preloads = if preload {
-            self.vite_manifest.preload_for(path).map_err(|_e| {
-                Error::new(
-                    ErrorKind::InvalidOperation,
-                    "Invalid assets manifest while calling function `include_asset`",
-                )
-            })?
-        } else {
-            BTreeSet::new()
-        };
+        let assets = std::iter::once(main)
+            .chain(imported.iter().filter(|a| a.is_stylesheet()).copied())
+            .filter_map(|asset| asset.include_tag(self.url_builder.assets_base().into()));
 
-        let preloads = preloads
+        let preloads = imported
             .iter()
-            // Only preload scripts and stylesheets for now
-            .filter(|asset| asset.is_script() || asset.is_stylesheet())
+            .filter(|a| a.is_script())
             .map(|asset| asset.preload_tag(self.url_builder.assets_base().into()));
 
-        let assets = assets
-            .iter()
-            .filter_map(|asset| asset.include_tag(self.url_builder.assets_base().into()));
-
         let tags: Vec<String> = preloads.chain(assets).collect();
 
         Ok(Value::from_safe_string(tags.join("\n")))

frontend/.storybook/preview.tsx

@@ -15,7 +15,7 @@
 import { ArgTypes, Decorator, Parameters, Preview } from "@storybook/react";
 import { useLayoutEffect } from "react";
 
-import "../src/main.css";
+import "../src/shared.css";
 import i18n from "../src/i18n";
 
 import localazyMetadata from "./locales";

frontend/index.html

@@ -1,5 +1,5 @@
 <!--
-Copyright 2022 The Matrix.org Foundation C.I.C.
+Copyright 2022-2024 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -26,19 +26,6 @@
       window.APP_CONFIG = JSON.parse(
         '{"root": "/account/", "graphqlEndpoint": "/graphql"}',
       );
-      (function () {
-        const query = window.matchMedia("(prefers-color-scheme: dark)");
-        function handleChange(list) {
-          if (list.matches) {
-            document.documentElement.classList.add("cpd-theme-dark");
-          } else {
-            document.documentElement.classList.remove("cpd-theme-dark");
-          }
-        }
-
-        query.addEventListener("change", handleChange);
-        handleChange(query);
-      })();
     </script>
   </head>
 

frontend/src/main.tsx

@@ -25,7 +25,7 @@ import config from "./config";
 import { client } from "./graphql";
 import i18n from "./i18n";
 import { routeTree } from "./routeTree.gen";
-import "./main.css";
+import "./shared.css";
 
 // Create a new router instance
 const router = createRouter({

frontend/src/main.css → frontend/src/shared.css

@@ -1,4 +1,4 @@
-/* Copyright 2022 The Matrix.org Foundation C.I.C.
+/* Copyright 2024 The Matrix.org Foundation C.I.C.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

frontend/src/templates.css

@@ -13,15 +13,6 @@
  * limitations under the License.
  */
 
-@import url("@fontsource/inter/400.css");
-@import url("@fontsource/inter/500.css");
-@import url("@fontsource/inter/600.css");
-@import url("@fontsource/inter/700.css");
-@import url("@fontsource/inconsolata/400.css");
-@import url("@fontsource/inconsolata/700.css");
-@import url("@vector-im/compound-design-tokens/assets/web/css/compound-design-tokens.css");
-@import url("@vector-im/compound-web/dist/style.css");
-
 @import url("./styles/cpd-button.css");
 @import url("./styles/cpd-form.css");
 @import url("./styles/cpd-link.css");
@@ -35,12 +26,6 @@
 @import url("./components/Footer/Footer.module.css");
 @import url("./components/PageHeading/PageHeading.module.css");
 
-@config "../tailwind.templates.config.cjs";
-
-@tailwind base;
-@tailwind components;
-@tailwind utilities;
-
 .cpd-text-body-lg-regular {
   font: var(--cpd-font-body-lg-regular);
   letter-spacing: var(--cpd-font-letter-spacing-body-lg);

frontend/tailwind.config.cjs

@@ -1,4 +1,4 @@
-// Copyright 2022 The Matrix.org Foundation C.I.C.
+// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -18,8 +18,7 @@
 
 module.exports = {
   mode: "jit",
-  content: ["./src/**/*.tsx", "./index.html"],
-  darkMode: ["class", ".cpd-theme-dark"],
+  content: ["./src/**/*.tsx", "./index.html", "../templates/**/*.html"],
   theme: {
     colors: {
       white: "#FFFFFF",

frontend/vite.config.ts

@@ -59,14 +59,12 @@ export default defineConfig((env) => ({
     sourcemap: true,
     modulePreload: false,
     target: browserslistToEsbuild(),
-
-    // We don't exactly handle CSS code splitting well if we're lazy loading components.
-    // We disabled lazy loading for now, but we should fix this at some point.
     cssCodeSplit: true,
 
     rollupOptions: {
       input: [
         resolve(__dirname, "src/main.tsx"),
+        resolve(__dirname, "src/shared.css"),
         resolve(__dirname, "src/templates.css"),
       ],
     },

templates/app.html

@@ -23,27 +23,14 @@
     <meta charset="UTF-8" />
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <title>{{ _("app.name") }}</title>
+    {% set config = {
+      'graphqlEndpoint': app_config.graphqlEndpoint,
+      'root': app_config.root,
+    } -%}
     <script>
-      {% set config = {
-        'graphqlEndpoint': app_config.graphqlEndpoint,
-        'root': app_config.root,
-      } -%}
       window.APP_CONFIG = JSON.parse("{{ config | tojson | add_slashes | safe }}");
-      (function () {
-        const query = window.matchMedia("(prefers-color-scheme: dark)");
-        function handleChange(list) {
-          if (list.matches) {
-            document.documentElement.classList.add("cpd-theme-dark");
-          } else {
-            document.documentElement.classList.remove("cpd-theme-dark");
-          }
-        }
-
-        query.addEventListener("change", handleChange);
-        handleChange(query);
-      })();
     </script>
-    {{ include_asset('src/main.tsx', preload=true) | indent(4) | safe }}
+    {{ include_asset('src/main.tsx') | indent(4) | safe }}
   </head>
 
   <body>

templates/base.html

@@ -31,7 +31,8 @@
     <meta charset="utf-8">
     <title>{% block title %}{{ _("app.name") }}{% endblock title %}</title>
     <meta name="viewport" content="width=device-width, initial-scale=1">
-    {{ include_asset('src/templates.css', preload=true) | indent(4) | safe }}
+    {{ include_asset('src/shared.css') | indent(4) | safe }}
+    {{ include_asset('src/templates.css') | indent(4) | safe }}
     {{ captcha.head() }}
   </head>
   <body>