From 19e2473fe1ff9a878a0e06587279f1fa9216ecf2 Mon Sep 17 00:00:00 2001 From: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com> Date: Sat, 29 Oct 2022 02:52:23 +0200 Subject: [PATCH 1/3] feat(cli): Load browserslist configuration if no `--targets` supplied. --- README.md | 40 ++++++++++++++++++++++++++++++++++++++++ src/main.rs | 3 ++- src/targets.rs | 33 +++++++++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index a9ad502c..2d80c0b8 100644 --- a/README.md +++ b/README.md @@ -197,6 +197,46 @@ To see all of the available options, use the `--help` argument: npx lightningcss --help ``` +#### Browserslist configuration + +If no `--targets` option is provided, then `lightningcss` finds browserslist configuration, +selects queries by environment and loads the resulting queries as targets. + +Configuration resolution is modeled after the original `browserslist` nodeJS package. +The configuration is resolved in the following order: + +- If a `BROWSERSLIST` environment variable is present, then load targets from its value. This is analog to the `--targets` CLI option. + _Example:_ `BROWSERSLIST="firefox ESR" lightningcss [OPTIONS] ` +- If a `BROWSERSLIST_CONFIG` environment variable is present, then resolve the file at the provided path. + Then parse and use targets from `package.json` or any browserslist configuration file pointed to by the environment variable. + _Example:_ `BROWSERSLIST_CONFIG="../config/browserslist" lightningcss [OPTIONS] ` +- If none of the above apply, then find, parse and use targets from the first `browserslist`, `package.json` + or `.browserslistrc` configuration file in any parent directory. + +Browserslist configuration files may contain sections denoted by angular brackets `[]`. +Use these to specify different targets for different environments. +Targets which are not placed in a section are added to `defaults` and used if no section applies matches the environment. + +_Example:_ + +``` +# Defaults, applied when no other section matches the provided environment. +firefox ESR + +[staging] +# Targets applied only to the staging environment. +samsung >= 4 +``` + +When using parsed configuration from `browserslist`, `package.json` or `.browserslistrc` configuration files, +the environment determined by + +- the `BROWSERSLIST_ENV` environment variable if present, +- otherwise the `NODE_ENV` environment variable if present, +- otherwise `production` is used. + +If no targets are found for the resulting environment, then the `defaults` configuration section is used. + ### Error recovery By default, Lightning CSS is strict, and will error when parsing an invalid rule or declaration. However, sometimes you may encounter a third party library that you can't easily modify, which unintentionally contains invalid syntax, or IE-specific hacks. In these cases, you can enable the `errorRecovery` option (or `--error-recovery` CLI flag). This will skip over invalid rules and declarations, omitting them in the output, and producing a warning instead of an error. You should also open an issue or PR to fix the issue in the library if possible. diff --git a/src/main.rs b/src/main.rs index e62f588c..fdabb0bf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -125,10 +125,11 @@ pub fn main() -> Result<(), std::io::Error> { }; let targets = if cli_args.targets.is_empty() { - None + Browsers::load_browserslist().unwrap() } else { Browsers::from_browserslist(cli_args.targets).unwrap() }; + stylesheet .minify(MinifyOptions { targets, diff --git a/src/targets.rs b/src/targets.rs index cddfc1ad..26c6649c 100644 --- a/src/targets.rs +++ b/src/targets.rs @@ -42,11 +42,40 @@ impl Browsers { ) -> Result, browserslist::Error> { use browserslist::{resolve, Opts}; - let res = resolve(query, &Opts::new())?; + Self::from_distribs(resolve(query, &Opts::new())?) + } + + /// Finds browserslist configuration, selects queries by environment and loads the resulting queries into LightningCSS targets. + /// + /// Configuration resolution is modeled after the original `browserslist` nodeJS package. + /// The configuration is resolved in the following order: + /// + /// - If a `BROWSERSLIST` environment variable is present, then load targets from its value. This is analog to the `--targets` CLI option. + /// Example: `BROWSERSLIST="firefox ESR" lightningcss [OPTIONS] ` + /// - If a `BROWSERSLIST_CONFIG` environment variable is present, then resolve the file at the provided path. + /// Then parse and use targets from `package.json` or any browserslist configuration file pointed to by the environment variable. + /// Example: `BROWSERSLIST_CONFIG="../config/browserslist" lightningcss [OPTIONS] ` + /// - If none of the above apply, then find, parse and use targets from the first `browserslist`, `package.json` + /// or `.browserslistrc` configuration file in any parent directory. + /// + /// When using parsed configuration from `browserslist`, `package.json` or `.browserslistrc` configuration files, + /// the environment determined by: + /// + /// - the `BROWSERSLIST_ENV` environment variable if present, + /// - otherwise the `NODE_ENV` environment varialbe if present, + /// - otherwise `production` is used. + /// + /// If no targets are found for the resulting environment, then the `defaults` configuration section is used. + pub fn load_browserslist() -> Result, browserslist::Error> { + use browserslist::{execute, Opts}; + + Self::from_distribs(execute(&Opts::new())?) + } + fn from_distribs(distribs: Vec) -> Result, browserslist::Error> { let mut browsers = Browsers::default(); let mut has_any = false; - for distrib in res { + for distrib in distribs { macro_rules! browser { ($browser: ident) => {{ if let Some(v) = parse_version(distrib.version()) { From d06231ae421a5946e341dab71691a02dd41449fa Mon Sep 17 00:00:00 2001 From: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com> Date: Sat, 29 Oct 2022 04:00:59 +0200 Subject: [PATCH 2/3] feat: Allow disabling browserslist, e.g. to preserve unused custom media --- README.md | 2 ++ src/main.rs | 10 +++++++--- tests/cli_integration_tests.rs | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 2d80c0b8..c24e77d1 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,8 @@ npx lightningcss --help If no `--targets` option is provided, then `lightningcss` finds browserslist configuration, selects queries by environment and loads the resulting queries as targets. +Pass the `--disable-browserslist` option if you do not want to load browserslist configuration or apply browserslist built-in defaults. + Configuration resolution is modeled after the original `browserslist` nodeJS package. The configuration is resolved in the following order: diff --git a/src/main.rs b/src/main.rs index fdabb0bf..72ec5011 100644 --- a/src/main.rs +++ b/src/main.rs @@ -46,6 +46,8 @@ struct CliArgs { #[clap(short, long, value_parser)] targets: Vec, #[clap(long, value_parser)] + disable_browserslist: bool, + #[clap(long, value_parser)] error_recovery: bool, } @@ -124,10 +126,12 @@ pub fn main() -> Result<(), std::io::Error> { StyleSheet::parse(&source, options).unwrap() }; - let targets = if cli_args.targets.is_empty() { - Browsers::load_browserslist().unwrap() - } else { + let targets = if !cli_args.targets.is_empty() { Browsers::from_browserslist(cli_args.targets).unwrap() + } else if cli_args.disable_browserslist { + None + } else { + Browsers::load_browserslist().unwrap() }; stylesheet diff --git a/tests/cli_integration_tests.rs b/tests/cli_integration_tests.rs index 832d3802..63ead8a9 100644 --- a/tests/cli_integration_tests.rs +++ b/tests/cli_integration_tests.rs @@ -379,6 +379,7 @@ fn preserve_custom_media() -> Result<(), Box> { let mut cmd = Command::cargo_bin("lightningcss")?; cmd.arg(file.path()); cmd.arg("--custom-media"); + cmd.arg("--disable-browserslist"); cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" @custom-media --foo print; "#})); From cb9a3e6d6d0c73bffe860b689a6f1936c481434f Mon Sep 17 00:00:00 2001 From: LeoniePhiline <22329650+LeoniePhiline@users.noreply.github.com> Date: Mon, 31 Oct 2022 21:53:30 +0100 Subject: [PATCH 3/3] feat: Make browserslist config discovery opt-in; add integration tests --- README.md | 17 +- src/main.rs | 14 +- src/targets.rs | 6 +- tests/cli_integration_tests.rs | 299 ++++++++++++++++++++++++++++++++- 4 files changed, 318 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index c24e77d1..ef9e70e0 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ An extremely fast CSS parser, transformer, and minifier written in Rust. Use it - Removing default property sub-values which will be inferred by browsers. - Many micro-optimizations, e.g. converting to shorter units, removing unnecessary quotation marks, etc. - **Vendor prefixing** – Lightning CSS accepts a list of browser targets, and automatically adds (and removes) vendor prefixes. +- **Browserslist configuration** – Lightning CSS supports opt-in browserslist configuration discovery to resolve browser targets and integrate with your existing tools and config setup. - **Syntax lowering** – Lightning CSS parses modern CSS syntax, and generates more compatible output where needed, based on browser targets. - CSS Nesting (draft spec) - Custom media queries (draft spec) @@ -199,12 +200,10 @@ npx lightningcss --help #### Browserslist configuration -If no `--targets` option is provided, then `lightningcss` finds browserslist configuration, +If the `--browserslist` option is provided, then `lightningcss` finds browserslist configuration, selects queries by environment and loads the resulting queries as targets. -Pass the `--disable-browserslist` option if you do not want to load browserslist configuration or apply browserslist built-in defaults. - -Configuration resolution is modeled after the original `browserslist` nodeJS package. +Configuration discovery and targets resolution is modeled after the original `browserslist` nodeJS package. The configuration is resolved in the following order: - If a `BROWSERSLIST` environment variable is present, then load targets from its value. This is analog to the `--targets` CLI option. @@ -212,8 +211,8 @@ The configuration is resolved in the following order: - If a `BROWSERSLIST_CONFIG` environment variable is present, then resolve the file at the provided path. Then parse and use targets from `package.json` or any browserslist configuration file pointed to by the environment variable. _Example:_ `BROWSERSLIST_CONFIG="../config/browserslist" lightningcss [OPTIONS] ` -- If none of the above apply, then find, parse and use targets from the first `browserslist`, `package.json` - or `.browserslistrc` configuration file in any parent directory. +- If none of the above apply, then find, parse and use targets from the first `browserslist`, `.browserslistrc` + or `package.json` configuration file in any parent directory. Browserslist configuration files may contain sections denoted by angular brackets `[]`. Use these to specify different targets for different environments. @@ -230,7 +229,7 @@ firefox ESR samsung >= 4 ``` -When using parsed configuration from `browserslist`, `package.json` or `.browserslistrc` configuration files, +When using parsed configuration from `browserslist`, `.browserslistrc` or `package.json` configuration files, the environment determined by - the `BROWSERSLIST_ENV` environment variable if present, @@ -249,7 +248,7 @@ By default, Lightning CSS is strict, and will error when parsing an invalid rule ``` -$ node bench.js bootstrap-4.css +$ node bench.js bootstrap-4.css cssnano: 544.809ms 159636 bytes @@ -271,7 +270,7 @@ lightningcss: 1.973ms 23666 bytes -$ node bench.js tailwind.css +$ node bench.js tailwind.css cssnano: 2.198s 1925626 bytes diff --git a/src/main.rs b/src/main.rs index 72ec5011..b95e9908 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,4 @@ -use clap::Parser; +use clap::{ArgGroup, Parser}; use lightningcss::bundler::{Bundler, FileProvider}; use lightningcss::stylesheet::{MinifyOptions, ParserOptions, PrinterOptions, StyleSheet}; use lightningcss::targets::Browsers; @@ -13,6 +13,10 @@ static GLOBAL: jemallocator::Jemalloc = jemallocator::Jemalloc; #[derive(Parser, Debug)] #[clap(author, version, about, long_about = None)] +#[clap(group( + ArgGroup::new("targets-resolution") + .args(&["targets", "browserslist"]), +))] struct CliArgs { /// Target CSS file #[clap(value_parser)] @@ -46,7 +50,7 @@ struct CliArgs { #[clap(short, long, value_parser)] targets: Vec, #[clap(long, value_parser)] - disable_browserslist: bool, + browserslist: bool, #[clap(long, value_parser)] error_recovery: bool, } @@ -128,10 +132,10 @@ pub fn main() -> Result<(), std::io::Error> { let targets = if !cli_args.targets.is_empty() { Browsers::from_browserslist(cli_args.targets).unwrap() - } else if cli_args.disable_browserslist { - None - } else { + } else if cli_args.browserslist { Browsers::load_browserslist().unwrap() + } else { + None }; stylesheet diff --git a/src/targets.rs b/src/targets.rs index 26c6649c..c87c92e1 100644 --- a/src/targets.rs +++ b/src/targets.rs @@ -55,10 +55,10 @@ impl Browsers { /// - If a `BROWSERSLIST_CONFIG` environment variable is present, then resolve the file at the provided path. /// Then parse and use targets from `package.json` or any browserslist configuration file pointed to by the environment variable. /// Example: `BROWSERSLIST_CONFIG="../config/browserslist" lightningcss [OPTIONS] ` - /// - If none of the above apply, then find, parse and use targets from the first `browserslist`, `package.json` - /// or `.browserslistrc` configuration file in any parent directory. + /// - If none of the above apply, then find, parse and use targets from the first `browserslist`, `.browserslistrc` + /// or `package.json` configuration file in any parent directory. /// - /// When using parsed configuration from `browserslist`, `package.json` or `.browserslistrc` configuration files, + /// When using parsed configuration from `browserslist`, `.browserslistrc` or `package.json` configuration files, /// the environment determined by: /// /// - the `BROWSERSLIST_ENV` environment variable if present, diff --git a/tests/cli_integration_tests.rs b/tests/cli_integration_tests.rs index 63ead8a9..2379b7e8 100644 --- a/tests/cli_integration_tests.rs +++ b/tests/cli_integration_tests.rs @@ -379,10 +379,307 @@ fn preserve_custom_media() -> Result<(), Box> { let mut cmd = Command::cargo_bin("lightningcss")?; cmd.arg(file.path()); cmd.arg("--custom-media"); - cmd.arg("--disable-browserslist"); cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" @custom-media --foo print; "#})); Ok(()) } + +#[test] +/// Test command line argument parsing failing when `--targets` is used at the same time as `--browserslist`. +/// The two options are mutually exclusive. +fn browserslist_targets_exclusive() -> Result<(), Box> { + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.arg("--targets").arg("defaults"); + cmd.arg("--browserslist"); + cmd.assert().failure().stderr(predicate::str::contains(indoc! {r#" + error: The argument '--targets ' cannot be used with '--browserslist' + "#})); + + Ok(()) +} + +#[test] +/// Test browserslist defaults being applied when no configuration is provided or discovered. +/// +/// Note: This test might fail in unhygienic environments and should ideally run inside a chroot. +/// We have no control over the contents of our temp dir's parent dir (e.g. `/tmp`). +/// If this parent dir or its ancestors contain a `browserslist`, `.browserslistrc` or `package.json` +/// file, then configuration will be read from there, instead of applying defaults. +fn browserslist_defaults() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test browserslist configuration being read from the `BROWSERSLIST` environment variable. +fn browserslist_env_config() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.env("BROWSERSLIST", "safari 4"); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test browserslist configuration being read from the file configured +/// by setting the `BROWSERSLIST_CONFIG` environment variable. +fn browserslist_env_config_file() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child("config"); + config.write_str( + r#" + safari 4 + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.env("BROWSERSLIST_CONFIG", config.path()); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test `browserslist` configuration file being read. +fn browserslist_config_discovery() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child("browserslist"); + config.write_str( + r#" + safari 4 + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test `.browserslistrc` configuration file being read. +fn browserslist_rc_discovery() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child(".browserslistrc"); + config.write_str( + r#" + safari 4 + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test `package.json` configuration section being read. +fn browserslist_package_discovery() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child("package.json"); + config.write_str( + r#" + { + "browserslist": "safari 4" + } + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test environment targets being applied from the `NODE_ENV` environment variable. +fn browserslist_environment_from_node_env() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child("browserslist"); + config.write_str( + r#" + last 1 Chrome version + + [legacy] + safari 4 + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.env("NODE_ENV", "legacy"); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +} + +#[test] +/// Test environment targets being applied from the `BROWSERSLIST_ENV` environment variable. +fn browserslist_environment_from_browserslist_env() -> Result<(), Box> { + let dir = assert_fs::TempDir::new()?; + let file = dir.child("test.css"); + file.write_str( + r#" + * { + border-radius: 1rem; + } + "#, + )?; + + let config = dir.child("browserslist"); + config.write_str( + r#" + last 1 Chrome version + + [legacy] + safari 4 + "#, + )?; + + let mut cmd = Command::cargo_bin("lightningcss")?; + cmd.current_dir(dir.path()); + cmd.env_clear(); + cmd.env("BROWSERSLIST_ENV", "legacy"); + cmd.arg("--browserslist"); + cmd.arg(file.path()); + cmd.assert().success().stdout(predicate::str::contains(indoc! {r#" + * { + -webkit-border-radius: 1rem; + border-radius: 1rem; + } + "#})); + + Ok(()) +}