Skip to content
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

Use a better default browser list and fail gracefully when css parsing fails #3509

Merged
merged 1 commit into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cli-opt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ png = "0.17.9"
image = { version = "0.25", features = ["avif"] }

# CSS Minification
lightningcss = "1.0.0-alpha.60"
lightningcss = { version = "1.0.0-alpha.60", features = ["browserslist", "into_owned"] }

# SCSS Processing
grass = "0.13.4"
Expand Down
54 changes: 44 additions & 10 deletions packages/cli-opt/src/css.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use grass::OutputStyle;
use lightningcss::{
printer::PrinterOptions,
stylesheet::{MinifyOptions, ParserOptions, StyleSheet},
targets::{Browsers, Targets},
};
use manganis_core::CssAssetOptions;
use tracing::{debug, warn};

pub(crate) fn process_css(
css_options: &CssAssetOptions,
Expand All @@ -18,7 +18,17 @@ pub(crate) fn process_css(
let css = std::fs::read_to_string(source)?;

let css = if css_options.minified() {
minify_css(&css)
// Try to minify the css. If we fail, log the error and use the unminified css
match minify_css(&css) {
Ok(minified) => minified,
Err(err) => {
tracing::error!(
"Failed to minify css; Falling back to unminified css. Error: {}",
err
);
css
}
}
} else {
css
};
Expand All @@ -33,15 +43,39 @@ pub(crate) fn process_css(
Ok(())
}

pub(crate) fn minify_css(css: &str) -> String {
let mut stylesheet = StyleSheet::parse(css, ParserOptions::default()).unwrap();
stylesheet.minify(MinifyOptions::default()).unwrap();
pub(crate) fn minify_css(css: &str) -> anyhow::Result<String> {
let options = ParserOptions {
error_recovery: true,
..Default::default()
};
let mut stylesheet = StyleSheet::parse(css, options).map_err(|err| err.into_owned())?;

// We load the browser list from the standard browser list file or use the browserslist default if we don't find any
// settings. Without the browser lists default, lightningcss will default to supporting only the newest versions of
// browsers.
let browsers_list = match Browsers::load_browserslist()? {
Some(browsers) => Some(browsers),
None => {
Browsers::from_browserslist(["defaults"]).expect("borwserslists should have defaults")
}
};

let targets = Targets {
browsers: browsers_list,
..Default::default()
};

stylesheet.minify(MinifyOptions {
targets,
..Default::default()
})?;
let printer = PrinterOptions {
targets,
minify: true,
..Default::default()
};
let res = stylesheet.to_css(printer).unwrap();
res.code
let res = stylesheet.to_css(printer)?;
Ok(res.code)
}

/// Process an scss/sass file into css.
Expand All @@ -61,7 +95,7 @@ pub(crate) fn process_scss(
.logger(&ScssLogger {});

let css = grass::from_path(source, &options)?;
let minified = minify_css(&css);
let minified = minify_css(&css)?;

std::fs::write(output_path, minified).with_context(|| {
format!(
Expand All @@ -79,7 +113,7 @@ struct ScssLogger {}

impl grass::Logger for ScssLogger {
fn debug(&self, location: SpanLoc, message: &str) {
debug!(
tracing::debug!(
"{}:{} DEBUG: {}",
location.file.name(),
location.begin.line + 1,
Expand All @@ -88,7 +122,7 @@ impl grass::Logger for ScssLogger {
}

fn warn(&self, location: SpanLoc, message: &str) {
warn!(
tracing::warn!(
"Warning: {}\n ./{}:{}:{}",
message,
location.file.name(),
Expand Down
10 changes: 9 additions & 1 deletion packages/cli/src/serve/handle.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{AppBundle, Platform, Result};
use anyhow::Context;
use dioxus_cli_opt::process_file_to;
use std::{
fs,
net::SocketAddr,
Expand Down Expand Up @@ -195,7 +196,14 @@ impl AppHandle {

// The asset might've been renamed thanks to the manifest, let's attempt to reload that too
if let Some(resource) = self.app.app.assets.assets.get(&changed_file).as_ref() {
let res = std::fs::copy(&changed_file, asset_dir.join(resource.bundled_path()));
let output_path = asset_dir.join(resource.bundled_path());
// Remove the old asset if it exists
_ = std::fs::remove_file(&output_path);
// And then process the asset with the options into the **old** asset location. If we recompiled,
// the asset would be in a new location because the contents and hash have changed. Since we are
// hotreloading, we need to use the old asset location it was originally written to.
let options = *resource.options();
let res = process_file_to(&options, &changed_file, &output_path);
bundled_name = Some(PathBuf::from(resource.bundled_path()));
if let Err(e) = res {
tracing::debug!("Failed to hotreload asset {e}");
Expand Down
Loading