From bb5bcf0f4010bd113e7961bf8f2aace8e266af8b Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Fri, 19 Jul 2024 11:57:46 +0200 Subject: [PATCH] Rework assets loading to fix splitting CSS chunks --- crates/spa/src/vite.rs | 79 +++++++++++--------------- crates/templates/src/functions.rs | 35 ++++-------- frontend/.storybook/preview.tsx | 2 +- frontend/index.html | 15 +---- frontend/src/main.tsx | 2 +- frontend/src/{main.css => shared.css} | 2 +- frontend/src/templates.css | 15 ----- frontend/tailwind.config.cjs | 5 +- frontend/tailwind.templates.config.cjs | 25 -------- frontend/vite.config.ts | 4 +- templates/app.html | 23 ++------ templates/base.html | 3 +- 12 files changed, 57 insertions(+), 153 deletions(-) rename frontend/src/{main.css => shared.css} (95%) delete mode 100644 frontend/tailwind.templates.config.cjs diff --git a/crates/spa/src/vite.rs b/crates/spa/src/vite.rs index 8a2ab6219..0f99a5531 100644 --- a/crates/spa/src/vite.rs +++ b/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>, + #[allow(dead_code)] dynamic_imports: Option>, integrity: Option, @@ -191,35 +192,24 @@ impl<'a> Asset<'a> { impl Manifest { /// Find all assets which should be loaded 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 assets_for<'a>( + pub fn find_assets<'a>( &'a self, entrypoint: &'a Utf8Path, - ) -> Result>, InvalidManifest<'a>> { + ) -> Result<(Asset<'a>, BTreeSet>), 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() - } + let mut entries = BTreeSet::new(); + let main_asset = self.find_imported_chunks(entry, &mut entries)?; - /// Find all assets which should be preloaded for a given entrypoint - /// - /// # Errors - /// - /// Returns an error if the entrypoint is invalid for this manifest - pub fn preload_for<'a>( - &'a self, - entrypoint: &'a Utf8Path, - ) -> Result>, InvalidManifest<'a>> { - let entry = self.lookup_by_name(entrypoint)?; - self.find_preload(entry) + // 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>, 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>, - ) -> Result<(), InvalidManifest<'a>> { + ) -> Result> { 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) = ¤t_entry.css { + for file in css { + 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(assets) = ¤t_entry.assets { + for file in assets { + let entry = self.lookup_by_file(file)?; + self.find_imported_chunks(entry, entries)?; + } + } + + if let Some(imports) = ¤t_entry.imports { + for import in imports { + let entry = self.lookup_by_name(import)?; + self.find_imported_chunks(entry, entries)?; + } } } - Ok(()) + Ok(asset) } } diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs index a447ad12e..9e97f21c3 100644 --- a/crates/templates/src/functions.rs +++ b/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, _state: &State, args: &[Value]) -> Result { - 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 preloads = preloads - .iter() - // Only preload scripts and stylesheets for now - .filter(|asset| asset.is_script() || asset.is_stylesheet()) - .map(|asset| asset.preload_tag(self.url_builder.assets_base().into())); - - let assets = assets - .iter() + 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 = imported + .iter() + .filter(|a| a.is_script()) + .map(|asset| asset.preload_tag(self.url_builder.assets_base().into())); + let tags: Vec = preloads.chain(assets).collect(); Ok(Value::from_safe_string(tags.join("\n"))) diff --git a/frontend/.storybook/preview.tsx b/frontend/.storybook/preview.tsx index 3982bf57b..f743c8055 100644 --- a/frontend/.storybook/preview.tsx +++ b/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"; diff --git a/frontend/index.html b/frontend/index.html index 97151e626..5f4a698a8 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -1,5 +1,5 @@