From 81e4fcbb7c4e08817c6081d3b9134edd41418e5b Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Mon, 30 May 2022 16:29:59 -0700 Subject: [PATCH 1/5] Update `SourceProvider` and `bundle()` to be `async` (#174) The two `SourceProvider` methods are now `async` which will give a hook for JavaScript to implement them and be called from the main thread. Everything is `async` now where possible and `bundle()` executes the `Future` synchronously to maintain its contract. Since it won't support custom JavaScript resolvers, there should never be a case where `bundle()` can't execute synchronously. Rayon doesn't seem to support `async` iterators, but also shouldn't be as necessary here. Removed it in order to call `async` APIs. --- Cargo.lock | 235 +++++++++++++++++++-------- Cargo.toml | 5 +- node/Cargo.toml | 1 + node/src/lib.rs | 13 +- src/bundler.rs | 351 ++++++++++++++++++++++++----------------- src/main.rs | 5 +- src/properties/font.rs | 1 - 7 files changed, 385 insertions(+), 226 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9907e99e..2c0c6794 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -57,6 +57,28 @@ dependencies = [ "tempfile", ] +[[package]] +name = "async-recursion" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2cda8f4bcc10624c4e85bc66b3f452cca98cfa5ca002dc83a16aad2367641bea" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "async-trait" +version = "0.1.56" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96cf8829f67d2eab0b2dfa42c5d0ef737e0724e4a82b01b3e292456202b19716" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "atty" version = "0.2.14" @@ -216,40 +238,6 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fb4a24b1aaf0fd0ce8b45161144d6f42cd91677fd5940fd431183eb023b3a2b8" -[[package]] -name = "crossbeam-channel" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e54ea8bc3fb1ee042f5aace6e3c6e025d3874866da222930f70ce62aceba0bfa" -dependencies = [ - "cfg-if", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-deque" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6455c0ca19f0d2fbf751b908d5c55c1f5cbc65e03c4225427254b46890bdde1e" -dependencies = [ - "cfg-if", - "crossbeam-epoch", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-epoch" -version = "0.9.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c00d6d2ea26e8b151d99093005cb442fb9a37aeaca582a03ec70946f49ab5ed9" -dependencies = [ - "cfg-if", - "crossbeam-utils", - "lazy_static", - "memoffset", - "scopeguard", -] - [[package]] name = "crossbeam-utils" version = "0.8.7" @@ -391,6 +379,95 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" +[[package]] +name = "futures" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f73fe65f54d1e12b726f517d3e2135ca3125a437b6d998caf1962961f7172d9e" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3083ce4b914124575708913bca19bfe887522d6e2e6d0952943f5eac4a74010" +dependencies = [ + "futures-core", + "futures-sink", +] + +[[package]] +name = "futures-core" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c09fd04b7e4073ac7156a9539b57a484a8ea920f79c7c675d05d289ab6110d3" + +[[package]] +name = "futures-executor" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9420b90cfa29e327d0429f19be13e7ddb68fa1cccb09d65e5706b8c7a749b8a6" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-io" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc4045962a5a5e935ee2fdedaa4e08284547402885ab326734432bed5d12966b" + +[[package]] +name = "futures-macro" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33c1e13800337f4d4d7a316bf45a567dbcb6ffe087f16424852d97e97a91f512" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "futures-sink" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21163e139fa306126e6eedaf49ecdb4588f939600f0b1e770f4205ee4b7fa868" + +[[package]] +name = "futures-task" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57c66a976bf5909d801bbef33416c41372779507e7a6b3a5e25e4749c58f776a" + +[[package]] +name = "futures-util" +version = "0.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d8b7abd5d659d9b90c8cba917f6ec750a74e2dc23902ef9cd4cc8c8b22e6036a" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-macro", + "futures-sink", + "futures-task", + "memchr", + "pin-project-lite", + "pin-utils", + "slab", +] + [[package]] name = "fxhash" version = "0.2.1" @@ -760,6 +837,8 @@ version = "1.0.0-alpha.27" dependencies = [ "assert_cmd", "assert_fs", + "async-recursion", + "async-trait", "bitflags", "browserslist-rs", "clap", @@ -767,6 +846,7 @@ dependencies = [ "cssparser", "dashmap", "data-encoding", + "futures", "indoc", "itertools", "jemallocator", @@ -775,10 +855,10 @@ dependencies = [ "parcel_sourcemap", "pathdiff", "predicates", - "rayon", "serde", "serde_json", "smallvec", + "tokio", ] [[package]] @@ -797,6 +877,7 @@ dependencies = [ "serde-wasm-bindgen", "serde_bytes", "serde_json", + "tokio", "wasm-bindgen", ] @@ -942,6 +1023,18 @@ dependencies = [ "siphasher", ] +[[package]] +name = "pin-project-lite" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0a7ae3ac2f1173085d398531c705756c94a4c56843785df85a60c1a0afac116" + +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "ppv-lite86" version = "0.2.16" @@ -1016,11 +1109,11 @@ checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" [[package]] name = "proc-macro2" -version = "1.0.36" +version = "1.0.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c7342d5883fbccae1cc37a2353b09c87c9b0f3afd73f5fb9bba687a1f733b029" +checksum = "c54b25569025b7fc9651de43004ae593a75ad88543b17178aa5e1b9c4f15f56f" dependencies = [ - "unicode-xid", + "unicode-ident", ] [[package]] @@ -1103,31 +1196,6 @@ dependencies = [ "rand_core", ] -[[package]] -name = "rayon" -version = "1.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c06aca804d41dbc8ba42dfd964f0d01334eceb64314b9ecf7c5fad5188a06d90" -dependencies = [ - "autocfg", - "crossbeam-deque", - "either", - "rayon-core", -] - -[[package]] -name = "rayon-core" -version = "1.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d78120e2c850279833f1dd3582f730c4ab53ed95aeaaaa862a2a5c71b1656d8e" -dependencies = [ - "crossbeam-channel", - "crossbeam-deque", - "crossbeam-utils", - "lazy_static", - "num_cpus", -] - [[package]] name = "redox_syscall" version = "0.2.10" @@ -1292,6 +1360,12 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a86232ab60fa71287d7f2ddae4a7073f6b7aac33631c3015abb556f08c6d0a3e" +[[package]] +name = "slab" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb703cfe953bccee95685111adeedb76fabe4e97549a58d16f03ea7b9367bb32" + [[package]] name = "smallvec" version = "1.8.0" @@ -1335,13 +1409,13 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" [[package]] name = "syn" -version = "1.0.86" +version = "1.0.96" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a65b3f4ffa0092e9887669db0eae07941f023991ab58ea44da8fe8e2d511c6b" +checksum = "0748dd251e24453cb8717f0354206b91557e4ec8703673a4b30208f2abaf1ebf" dependencies = [ "proc-macro2", "quote", - "unicode-xid", + "unicode-ident", ] [[package]] @@ -1419,10 +1493,33 @@ dependencies = [ ] [[package]] -name = "unicode-xid" -version = "0.2.2" +name = "tokio" +version = "1.19.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95eec79ea28c00a365f539f1961e9278fbcaf81c0ff6aaf0e93c181352446948" +dependencies = [ + "num_cpus", + "once_cell", + "pin-project-lite", + "tokio-macros", +] + +[[package]] +name = "tokio-macros" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9724f9a975fb987ef7a3cd9be0350edcbe130698af5b8f7a631e23d42d052484" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "unicode-ident" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" +checksum = "d22af068fba1eb5edcb4aea19d382b2a3deb4c8f9d475c589b6ada9e0fd493ee" [[package]] name = "unindent" diff --git a/Cargo.toml b/Cargo.toml index 522091f9..47bc71fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,9 @@ path = "src/lib.rs" crate-type = ["rlib"] [dependencies] +async-recursion = "1.0.0" +async-trait = "0.1.53" +futures = "0.3.21" serde = { version = "1.0.123", features = ["derive"] } cssparser = "0.29.1" parcel_selectors = { version = "0.24.4", path = "./selectors" } @@ -35,12 +38,12 @@ parcel_sourcemap = "2.0.2" data-encoding = "2.3.2" lazy_static = "1.4.0" const-str = "0.3.1" +tokio = { version = "1.19.1", features = ["macros", "rt", "rt-multi-thread"] } # CLI deps clap = { version = "3.0.6", features = ["derive"], optional = true } serde_json = { version = "1.0.78", optional = true } pathdiff = { version = "0.2.1", optional = true } browserslist-rs = { version = "0.7.0", optional = true } -rayon = "1.5.1" dashmap = "5.0.0" [target.'cfg(target_os = "macos")'.dependencies] diff --git a/node/Cargo.toml b/node/Cargo.toml index 2be5cb8b..5d34cbb2 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -22,6 +22,7 @@ jemallocator = { version = "0.3.2", features = ["disable_initial_exec_tls"] } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] napi = {version = "2.2.0", default-features = false, features = ["napi4", "compat-mode", "serde-json"]} napi-derive = "2" +tokio = {version = "1.19.0", features = ["rt", "rt-multi-thread"]} [target.'cfg(target_arch = "wasm32")'.dependencies] js-sys = "0.3" diff --git a/node/src/lib.rs b/node/src/lib.rs index 407725d6..5ffb32fa 100644 --- a/node/src/lib.rs +++ b/node/src/lib.rs @@ -14,6 +14,7 @@ use parcel_sourcemap::SourceMap; use serde::{Deserialize, Serialize}; use std::collections::HashSet; use std::path::Path; +use tokio::runtime::Runtime; // --------------------------------------------- @@ -129,7 +130,8 @@ fn bundle(ctx: CallContext) -> napi::Result { let opts = ctx.get::(0)?; let config: BundleConfig = ctx.env.from_js_value(opts)?; let fs = FileProvider::new(); - let res = compile_bundle(&fs, &config); + let async_runtime = Runtime::new()?; + let res = async_runtime.block_on(compile_bundle(&fs, &config)); match res { Ok(res) => res.into_js(ctx), @@ -150,7 +152,7 @@ fn bundle(ctx: CallContext) -> napi::Result { | CompileError::BundleError(Error { loc: Some(ErrorLocation { filename, .. }), .. - }) => Some(fs.read(Path::new(filename))?), + }) => Some(async_runtime.block_on(fs.read(Path::new(filename)))?), _ => None, }; err.throw(ctx, code) @@ -313,7 +315,10 @@ fn compile<'i>(code: &'i str, config: &Config) -> Result(fs: &'i FileProvider, config: &BundleConfig) -> Result> { +async fn compile_bundle<'i>( + fs: &'i FileProvider, + config: &BundleConfig, +) -> Result> { let mut source_map = if config.source_map.unwrap_or(false) { Some(SourceMap::new("/")) } else { @@ -347,7 +352,7 @@ fn compile_bundle<'i>(fs: &'i FileProvider, config: &BundleConfig) -> Result { /// /// See [FileProvider](FileProvider) for an implementation that uses the /// file system. +#[async_trait] pub trait SourceProvider: Send + Sync { /// Reads the contents of the given file path to a string. - fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str>; + async fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str>; /// Resolves the given import specifier to a file path given the file /// which the import originated from. - fn resolve(&self, specifier: &str, originating_file: &Path) -> Result>; + async fn resolve(&self, specifier: &str, originating_file: &Path) -> Result>; } /// Provides an implementation of [SourceProvider](SourceProvider) @@ -105,8 +111,9 @@ impl FileProvider { unsafe impl Sync for FileProvider {} unsafe impl Send for FileProvider {} +#[async_trait] impl SourceProvider for FileProvider { - fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { + async fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { let source = fs::read_to_string(file)?; let ptr = Box::into_raw(Box::new(source)); self.inputs.lock().unwrap().push(ptr); @@ -116,7 +123,7 @@ impl SourceProvider for FileProvider { Ok(unsafe { &*ptr }) } - fn resolve(&self, specifier: &str, originating_file: &Path) -> Result> { + async fn resolve(&self, specifier: &str, originating_file: &Path) -> Result> { // Assume the specifier is a releative file path and join it with current path. Ok(originating_file.with_file_name(specifier)) } @@ -190,22 +197,24 @@ impl<'a, 'o, 's, P: SourceProvider> Bundler<'a, 'o, 's, P> { } /// Bundles the given entry file and all dependencies into a single style sheet. - pub fn bundle<'e>(&mut self, entry: &'e Path) -> Result, Error>> { + pub async fn bundle<'e>(&mut self, entry: &'e Path) -> Result, Error>> { // Phase 1: load and parse all files. This is done in parallel. - self.load_file( - &entry, - ImportRule { - url: "".into(), - layer: None, - supports: None, - media: MediaList::new(), - loc: Location { - source_index: 0, - line: 0, - column: 1, + self + .load_file( + &entry, + ImportRule { + url: "".into(), + layer: None, + supports: None, + media: MediaList::new(), + loc: Location { + source_index: 0, + line: 0, + column: 1, + }, }, - }, - )?; + ) + .await?; // Phase 2: determine the order that the files should be concatenated. self.order(); @@ -231,79 +240,84 @@ impl<'a, 'o, 's, P: SourceProvider> Bundler<'a, 'o, 's, P> { entry.key().to_str().unwrap().into() } - fn load_file(&self, file: &Path, rule: ImportRule<'a>) -> Result>> { - // Check if we already loaded this file. - let mut stylesheets = self.stylesheets.lock().unwrap(); - let source_index = match self.source_indexes.get(file) { - Some(source_index) => { - // If we already loaded this file, combine the media queries and supports conditions - // from this import rule with the existing ones using a logical or operator. - let entry = &mut stylesheets[*source_index as usize]; - - // We cannot combine a media query and a supports query from different @import rules. - // e.g. @import "a.css" print; @import "a.css" supports(color: red); - // This would require duplicating the actual rules in the file. - if (!rule.media.media_queries.is_empty() && !entry.supports.is_none()) - || (!entry.media.media_queries.is_empty() && !rule.supports.is_none()) - { - return Err(Error { - kind: BundleErrorKind::UnsupportedImportCondition, - loc: Some(ErrorLocation::new(rule.loc, self.find_filename(rule.loc.source_index))), - }); - } - - if rule.media.media_queries.is_empty() { - entry.media.media_queries.clear(); - } else if !entry.media.media_queries.is_empty() { - entry.media.or(&rule.media); - } + #[async_recursion] + async fn load_file(&self, file: &Path, rule: ImportRule<'a>) -> Result>> { + let source_index = { + // Check if we already loaded this file. + let mut stylesheets = self.stylesheets.lock().unwrap(); + let source_index = match self.source_indexes.get(file) { + Some(source_index) => { + // If we already loaded this file, combine the media queries and supports conditions + // from this import rule with the existing ones using a logical or operator. + let entry = &mut stylesheets[*source_index as usize]; + + // We cannot combine a media query and a supports query from different @import rules. + // e.g. @import "a.css" print; @import "a.css" supports(color: red); + // This would require duplicating the actual rules in the file. + if (!rule.media.media_queries.is_empty() && !entry.supports.is_none()) + || (!entry.media.media_queries.is_empty() && !rule.supports.is_none()) + { + return Err(Error { + kind: BundleErrorKind::UnsupportedImportCondition, + loc: Some(ErrorLocation::new(rule.loc, self.find_filename(rule.loc.source_index))), + }); + } - if let Some(supports) = rule.supports { - if let Some(existing_supports) = &mut entry.supports { - existing_supports.or(&supports) + if rule.media.media_queries.is_empty() { + entry.media.media_queries.clear(); + } else if !entry.media.media_queries.is_empty() { + entry.media.or(&rule.media); } - } else { - entry.supports = None; - } - if let Some(layer) = &rule.layer { - if let Some(existing_layer) = &entry.layer { - // We can't OR layer names without duplicating all of the nested rules, so error for now. - if layer != existing_layer || (layer.is_none() && existing_layer.is_none()) { - return Err(Error { - kind: BundleErrorKind::UnsupportedLayerCombination, - loc: Some(ErrorLocation::new(rule.loc, self.find_filename(rule.loc.source_index))), - }); + if let Some(supports) = rule.supports { + if let Some(existing_supports) = &mut entry.supports { + existing_supports.or(&supports) } } else { - entry.layer = rule.layer; + entry.supports = None; + } + + if let Some(layer) = &rule.layer { + if let Some(existing_layer) = &entry.layer { + // We can't OR layer names without duplicating all of the nested rules, so error for now. + if layer != existing_layer || (layer.is_none() && existing_layer.is_none()) { + return Err(Error { + kind: BundleErrorKind::UnsupportedLayerCombination, + loc: Some(ErrorLocation::new(rule.loc, self.find_filename(rule.loc.source_index))), + }); + } + } else { + entry.layer = rule.layer; + } } + + return Ok(*source_index); } + None => { + let source_index = stylesheets.len() as u32; + self.source_indexes.insert(file.to_owned(), source_index); + + stylesheets.push(BundleStyleSheet { + stylesheet: None, + layer: rule.layer.clone(), + media: rule.media.clone(), + supports: rule.supports.clone(), + loc: rule.loc.clone(), + dependencies: Vec::new(), + parent_source_index: 0, + parent_dep_index: 0, + }); - return Ok(*source_index); - } - None => { - let source_index = stylesheets.len() as u32; - self.source_indexes.insert(file.to_owned(), source_index); - - stylesheets.push(BundleStyleSheet { - stylesheet: None, - layer: rule.layer.clone(), - media: rule.media.clone(), - supports: rule.supports.clone(), - loc: rule.loc.clone(), - dependencies: Vec::new(), - parent_source_index: 0, - parent_dep_index: 0, - }); - - source_index - } - }; + source_index + } + }; - drop(stylesheets); // ensure we aren't holding the lock anymore + // Return just `source_index` so `stylesheets` falls out of scope and doesn't cross + // an `await` boundary, which would violate `Send` requirements. + source_index + }; - let code = self.fs.read(file).map_err(|e| Error { + let code = self.fs.read(file).await.map_err(|e| Error { kind: BundleErrorKind::IOError(e), loc: Some(ErrorLocation::new(rule.loc, self.find_filename(rule.loc.source_index))), })?; @@ -321,21 +335,21 @@ impl<'a, 'o, 's, P: SourceProvider> Bundler<'a, 'o, 's, P> { let mut stylesheet = StyleSheet::parse(filename, code, opts)?; // Collect and load dependencies for this stylesheet in parallel. - let dependencies: Result, _> = stylesheet - .rules - .0 - .par_iter_mut() - .filter_map(|r| { + let dependencies: Result, _> = future::join_all(stylesheet.rules.0.iter_mut().map(|r| async { + // Wrap in an async-compatible Mutex so we can easily pass it across `await` boundaries. + let mutex = futures::lock::Mutex::new(r); + let result = match mutex.lock().await.deref_mut() { // Prepend parent layer name to @layer statements. - if let CssRule::LayerStatement(layer) = r { + CssRule::LayerStatement(layer) => { if let Some(Some(parent_layer)) = &rule.layer { for name in &mut layer.names { name.0.insert_many(0, parent_layer.0.iter().cloned()) } } - } - if let CssRule::Import(import) = r { + None + } + CssRule::Import(import) => { let specifier = &import.url; // Combine media queries and supports conditions from parent @@ -376,17 +390,21 @@ impl<'a, 'o, 's, P: SourceProvider> Bundler<'a, 'o, 's, P> { import.layer.clone() }; - let result = match self.fs.resolve(&specifier, file) { - Ok(path) => self.load_file( - &path, - ImportRule { - layer, - media, - supports: combine_supports(rule.supports.clone(), &import.supports), - url: "".into(), - loc: import.loc, - }, - ), + let result = match self.fs.resolve(&specifier, file).await { + Ok(path) => { + self + .load_file( + &path, + ImportRule { + layer, + media, + supports: combine_supports(rule.supports.clone(), &import.supports), + url: "".into(), + loc: import.loc, + }, + ) + .await + } Err(err) => Err(Error { kind: err.kind, loc: Some(ErrorLocation::new( @@ -397,11 +415,15 @@ impl<'a, 'o, 's, P: SourceProvider> Bundler<'a, 'o, 's, P> { }; Some(result) - } else { - None } - }) - .collect(); + _ => None, + }; + result + })) + .await + .into_iter() + .filter_map(|value| value) + .collect(); let entry = &mut self.stylesheets.lock().unwrap()[source_index as usize]; entry.stylesheet = Some(stylesheet); @@ -529,12 +551,13 @@ mod tests { map: HashMap, } + #[async_trait] impl SourceProvider for TestProvider { - fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { + async fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { Ok(self.map.get(file).unwrap()) } - fn resolve(&self, specifier: &str, originating_file: &Path) -> Result> { + async fn resolve(&self, specifier: &str, originating_file: &Path) -> Result> { Ok(originating_file.with_file_name(specifier)) } } @@ -544,15 +567,16 @@ mod tests { map: HashMap, } + #[async_trait] impl SourceProvider for CustomProvider { /// Read files from in-memory map. - fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { + async fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { Ok(self.map.get(file).unwrap()) } /// Resolve by stripping a `foo:` prefix off any import. Specifiers without /// this prefix fail with an error. - fn resolve(&self, specifier: &str, _originating_file: &Path) -> Result> { + async fn resolve(&self, specifier: &str, _originating_file: &Path) -> Result> { if specifier.starts_with("foo:") { Ok(Path::new(&specifier["foo:".len()..]).to_path_buf()) } else { @@ -582,13 +606,13 @@ mod tests { }; ); - fn bundle(fs: P, entry: &str) -> String { + async fn bundle(fs: P, entry: &str) -> String { let mut bundler = Bundler::new(&fs, None, ParserOptions::default()); - let stylesheet = bundler.bundle(Path::new(entry)).unwrap(); + let stylesheet = bundler.bundle(Path::new(entry)).await.unwrap(); stylesheet.to_css(PrinterOptions::default()).unwrap().code } - fn bundle_css_module(fs: P, entry: &str) -> String { + async fn bundle_css_module(fs: P, entry: &str) -> String { let mut bundler = Bundler::new( &fs, None, @@ -597,11 +621,11 @@ mod tests { ..ParserOptions::default() }, ); - let stylesheet = bundler.bundle(Path::new(entry)).unwrap(); + let stylesheet = bundler.bundle(Path::new(entry)).await.unwrap(); stylesheet.to_css(PrinterOptions::default()).unwrap().code } - fn bundle_custom_media(fs: P, entry: &str) -> String { + async fn bundle_custom_media(fs: P, entry: &str) -> String { let mut bundler = Bundler::new( &fs, None, @@ -610,7 +634,7 @@ mod tests { ..ParserOptions::default() }, ); - let mut stylesheet = bundler.bundle(Path::new(entry)).unwrap(); + let mut stylesheet = bundler.bundle(Path::new(entry)).await.unwrap(); let targets = Some(Browsers { safari: Some(13 << 16), ..Browsers::default() @@ -630,9 +654,13 @@ mod tests { .code } - fn error_test(fs: P, entry: &str, maybe_cb: Option ()>>) { + async fn error_test( + fs: P, + entry: &str, + maybe_cb: Option ()>>, + ) { let mut bundler = Bundler::new(&fs, None, ParserOptions::default()); - let res = bundler.bundle(Path::new(entry)); + let res = bundler.bundle(Path::new(entry)).await; match res { Ok(_) => unreachable!(), Err(e) => { @@ -643,8 +671,8 @@ mod tests { } } - #[test] - fn test_bundle() { + #[tokio::test] + async fn test_bundle() { let res = bundle( TestProvider { map: fs! { @@ -658,7 +686,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -685,7 +714,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -714,7 +744,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -743,7 +774,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -775,7 +807,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -805,7 +838,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -838,7 +872,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -877,7 +912,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -904,7 +940,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -931,7 +968,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -958,7 +996,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -991,7 +1030,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1020,7 +1060,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1049,7 +1090,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1082,7 +1124,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1117,7 +1160,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1160,7 +1204,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1205,7 +1250,8 @@ mod tests { Some(Box::new(|err| { assert!(matches!(err, BundleErrorKind::UnsupportedLayerCombination)); })), - ); + ) + .await; error_test( TestProvider { @@ -1223,7 +1269,8 @@ mod tests { Some(Box::new(|err| { assert!(matches!(err, BundleErrorKind::UnsupportedLayerCombination)); })), - ); + ) + .await; error_test( TestProvider { @@ -1245,7 +1292,8 @@ mod tests { Some(Box::new(|err| { assert!(matches!(err, BundleErrorKind::UnsupportedLayerCombination)); })), - ); + ) + .await; error_test( TestProvider { @@ -1267,7 +1315,8 @@ mod tests { Some(Box::new(|err| { assert!(matches!(err, BundleErrorKind::UnsupportedLayerCombination)); })), - ); + ) + .await; let res = bundle( TestProvider { @@ -1293,7 +1342,8 @@ mod tests { }, }, "/index.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1329,7 +1379,8 @@ mod tests { }, }, "/index.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1354,7 +1405,8 @@ mod tests { }, }, "/a.css", - ); + ) + .await; assert_eq!( res, indoc! { r#" @@ -1390,7 +1442,8 @@ mod tests { .to_string() .contains("Failed to resolve `/b.css`, specifier does not start with `foo:`.")); })), - ); + ) + .await; // let res = bundle(fs! { // "/a.css": r#" @@ -1401,7 +1454,7 @@ mod tests { // "/b.css": r#" // .b { color: green } // "# - // }, "/a.css"); + // }, "/a.css").await; // let res = bundle(fs! { // "/a.css": r#" @@ -1415,6 +1468,6 @@ mod tests { // "/c.css": r#" // .c { color: yellow } // "# - // }, "/a.css"); + // }, "/a.css").await; } } diff --git a/src/main.rs b/src/main.rs index 5c2c696f..3d3aac73 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,7 +55,8 @@ struct SourceMapJson<'a> { names: &'a Vec, } -pub fn main() -> Result<(), std::io::Error> { +#[tokio::main] +pub async fn main() -> Result<(), std::io::Error> { let cli_args = CliArgs::parse(); let source = fs::read_to_string(&cli_args.input_file)?; @@ -101,7 +102,7 @@ pub fn main() -> Result<(), std::io::Error> { let mut stylesheet = if cli_args.bundle { let mut bundler = Bundler::new(&fs, source_map.as_mut(), options); - bundler.bundle(Path::new(&cli_args.input_file)).unwrap() + bundler.bundle(Path::new(&cli_args.input_file)).await.unwrap() } else { if let Some(sm) = &mut source_map { sm.add_source(&filename); diff --git a/src/properties/font.rs b/src/properties/font.rs index c87afe39..d4325640 100644 --- a/src/properties/font.rs +++ b/src/properties/font.rs @@ -932,7 +932,6 @@ const DEFAULT_SYSTEM_FONTS: &[&str] = &[ /// This list is an attempt at providing that support #[inline] fn compatible_font_family(mut family: Option>, is_supported: bool) -> Option> { - if is_supported { return family; } From 477bb6349227ce071bde444e7334fb9ba4e661ee Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Mon, 30 May 2022 14:41:37 -0700 Subject: [PATCH 2/5] Add `bundleAsync()` JS binding (#174) This is roughly equivalent to `bundle()`, except that is executes the `Future` *asynchronously*, returning a JS `Promise` holding the result. Errors are formatted a bit differently since they just use the `Display` trait to convert to strings, rather than the `throw()` function which converts them to a JS `SyntaxError` in `bundle()`. This can't be used in the async version because it must return a `napi::Result` which is immediately emitted to the user and no JS values are accessible. While it is possible to return the `CompileError` directly from the async block in a `napi::Result>` and then call `throw()` in the callback with access to JS data, doing so causes lifetime issues with `fs` and isn't easily viable. --- Cargo.lock | 1 + node/Cargo.toml | 2 +- node/src/lib.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 2c0c6794..63bb1033 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -720,6 +720,7 @@ dependencies = [ "serde", "serde_json", "thread_local", + "tokio", ] [[package]] diff --git a/node/Cargo.toml b/node/Cargo.toml index 5d34cbb2..638e3d96 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -20,7 +20,7 @@ parcel_sourcemap = "2.0.2" jemallocator = { version = "0.3.2", features = ["disable_initial_exec_tls"] } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -napi = {version = "2.2.0", default-features = false, features = ["napi4", "compat-mode", "serde-json"]} +napi = {version = "2.2.0", default-features = false, features = ["napi4", "compat-mode", "serde-json", "tokio_rt"]} napi-derive = "2" tokio = {version = "1.19.0", features = ["rt", "rt-multi-thread"]} diff --git a/node/src/lib.rs b/node/src/lib.rs index 5ffb32fa..b5bb5e6e 100644 --- a/node/src/lib.rs +++ b/node/src/lib.rs @@ -160,12 +160,41 @@ fn bundle(ctx: CallContext) -> napi::Result { } } +#[cfg(not(target_arch = "wasm32"))] +#[js_function(1)] +fn bundle_async(ctx: CallContext) -> napi::Result { + // Parse JS arguments into Rust values so they can be used in the `Future`. + let opts = ctx.get::(0)?; + let config: BundleConfig = ctx.env.from_js_value(&opts)?; + + // Execute asynchronous operation and return a `Promise` to JS. + ctx.env.execute_tokio_future( + // Perform asynchronous work, *cannot* access JS data from here. + async { + let fs = FileProvider::new(); + let res = compile_bundle(&fs, &config).await; + drop(config); + + match res { + Ok(transform_result) => Ok(transform_result), + Err(compile_error) => Err(napi::Error::new( + napi::Status::GenericFailure, + compile_error.to_string(), + )), + } + }, + // Convert the result from Rust data to JS data. + |&mut env, transform_result| env.to_js_value(&transform_result) + ) +} + #[cfg(not(target_arch = "wasm32"))] #[module_exports] fn init(mut exports: JsObject) -> napi::Result<()> { exports.create_named_method("transform", transform)?; exports.create_named_method("transformStyleAttribute", transform_style_attribute)?; exports.create_named_method("bundle", bundle)?; + exports.create_named_method("bundleAsync", bundle_async)?; Ok(()) } From de11ce6b71c92613d38fddbc5a822880ef30b316 Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Sat, 4 Jun 2022 22:29:59 -0700 Subject: [PATCH 3/5] Add `JsSourceProvider` and use it in `bundleAsync()` (#174) This adds a new `JsSourceProvider` which acts a `SourceProvider` which invokes associated JS implementations of the resolver if they exist or falls back to `FileProvider` when not given. This allows JS consumers to override one or both of these methods. If JS does *not* override either, they should not pay any significant performance penalty since all the Rust work will stay on the same thread. The JS implementations are invoked as thread-safe functions, pushing the arguments onto a queue and adding it to the event queue. Some time later, the main thread pulls from this queue and invokes the function. `napi-rs` doesn't seem to provide any means of receiving a JS return value in Rust, so instead arguments for both `read()` and `resolve()` include a callback function using Node calling conventions (`callback(err, result)`). This looks like: ```javascript await bundleAsync({ // Options... resolver: { read(file: string, cb: (string) => void): void { // Read file and invoke callback with result. fs.read(file, 'utf-8').then((res) => cb(null, res), (err) => cb(err, null)); }, resolve(specifier: string, originatingFile: string, cb: (string) => void): void { // Resolve and invoke callback with result. const resolved = path.resolve(originatingFile, '..', specifier); cb(null, resolved); }, }, }); ``` --- Cargo.lock | 12 ++ node/Cargo.toml | 5 +- node/src/lib.rs | 320 ++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 329 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63bb1033..584ea735 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -149,6 +149,15 @@ version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +[[package]] +name = "callback-future" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b76a63a41df0ba9490dd817a3d98511c6d3d9e530eff9bcf5f3de2a6723d1c3f" +dependencies = [ + "futures", +] + [[package]] name = "cc" version = "1.0.72" @@ -866,7 +875,10 @@ dependencies = [ name = "parcel_css_node" version = "0.1.0" dependencies = [ + "async-trait", + "callback-future", "cssparser", + "futures", "jemallocator", "js-sys", "napi", diff --git a/node/Cargo.toml b/node/Cargo.toml index 638e3d96..872d6b6a 100644 --- a/node/Cargo.toml +++ b/node/Cargo.toml @@ -20,7 +20,10 @@ parcel_sourcemap = "2.0.2" jemallocator = { version = "0.3.2", features = ["disable_initial_exec_tls"] } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -napi = {version = "2.2.0", default-features = false, features = ["napi4", "compat-mode", "serde-json", "tokio_rt"]} +async-trait = "0.1.53" +callback-future = "0.1" +futures = "0.3.21" +napi = {version = "2.2.0", default-features = false, features = ["napi5", "compat-mode", "serde-json", "tokio_rt"]} napi-derive = "2" tokio = {version = "1.19.0", features = ["rt", "rt-multi-thread"]} diff --git a/node/src/lib.rs b/node/src/lib.rs index b5bb5e6e..d6b321dc 100644 --- a/node/src/lib.rs +++ b/node/src/lib.rs @@ -2,6 +2,7 @@ #[global_allocator] static GLOBAL: jemallocator::Jemalloc = jemallocator::Jemalloc; +use async_trait::async_trait; use parcel_css::bundler::{BundleErrorKind, Bundler, FileProvider, SourceProvider}; use parcel_css::css_modules::{CssModuleExports, CssModuleReferences, PatternParseError}; use parcel_css::dependencies::Dependency; @@ -12,10 +13,15 @@ use parcel_css::stylesheet::{ use parcel_css::targets::Browsers; use parcel_sourcemap::SourceMap; use serde::{Deserialize, Serialize}; +use std::cell::Cell; use std::collections::HashSet; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::sync::Mutex; use tokio::runtime::Runtime; +use callback_future::CallbackFuture; + // --------------------------------------------- #[cfg(target_arch = "wasm32")] @@ -46,7 +52,11 @@ pub fn transform_style_attribute(config_val: JsValue) -> Result napi::Result { let opts = ctx.get::(0)?; let config: BundleConfig = ctx.env.from_js_value(&opts)?; + // Get `read()` and `resolve()` JS functions. + let maybe_resolver = &opts.get::<&str, JsObject>("resolver")?; + let maybe_unsafe_read = match &maybe_resolver { + None => None, + Some(resolver) => match resolver.get::<&str, JsUnknown>("read")? { + None => None, + Some(read) => { + let read_type = read.get_type().unwrap(); + if read_type != napi::ValueType::Function { + return Err(napi::Error::new( + napi::Status::FunctionExpected, + format!( + "Expected `resolver.read` to be of type `{}` but was of type `{}`", + napi::ValueType::Function, + read_type + ), + )); + } + + Some(unsafe { read.cast::() }) + } + }, + }; + let maybe_unsafe_resolve = match &maybe_resolver { + None => None, + Some(resolver) => match resolver.get::<&str, JsUnknown>("resolve")? { + None => None, + Some(resolve) => { + let resolve_type = resolve.get_type().unwrap(); + if resolve_type != napi::ValueType::Function { + return Err(napi::Error::new( + napi::Status::FunctionExpected, + format!( + "Expected `resolver.resolve` to be of type `{}` but was of type `{}`.", + napi::ValueType::Function, + resolve_type + ), + )); + } + + Some(unsafe { resolve.cast::() }) + } + }, + }; + + // Map `read()` and `resolve()` to thread-safe N-API functions. + let maybe_read: Option> = match maybe_unsafe_read { + None => None, + Some(unsafe_read) => Some(unsafe_read.create_threadsafe_function( + 0, /* max_queue_size */ + // On the main thread, convert Rust args struct into JS arguments. + |ctx: ThreadSafeCallContext| { + Ok(vec![ + ctx.env.create_string(&ctx.value.file.to_str().unwrap())?.into_unknown(), + ctx + .env + .create_function_from_closure("callback", ctx.value.callback)? + .into_unknown(), + ]) + }, + )?), + }; + let maybe_resolve = match maybe_unsafe_resolve { + None => None, + Some(unsafe_resolve) => Some(unsafe_resolve.create_threadsafe_function( + 0, /* max_queue_size */ + // On the main thread, convert Rust args struct into JS arguments. + |ctx: ThreadSafeCallContext| { + Ok(vec![ + ctx.env.create_string(&ctx.value.specifier)?.into_unknown(), + ctx + .env + .create_string(ctx.value.originating_file.to_str().unwrap())? + .into_unknown(), + ctx + .env + .create_function_from_closure("callback", ctx.value.callback)? + .into_unknown(), + ]) + }, + )?), + }; + // Execute asynchronous operation and return a `Promise` to JS. ctx.env.execute_tokio_future( // Perform asynchronous work, *cannot* access JS data from here. async { - let fs = FileProvider::new(); - let res = compile_bundle(&fs, &config).await; + let source_provider = JsSourceProvider::new(FileProvider::new(), maybe_read, maybe_resolve); + let res = compile_bundle(&source_provider, &config).await; drop(config); match res { @@ -184,10 +277,223 @@ fn bundle_async(ctx: CallContext) -> napi::Result { } }, // Convert the result from Rust data to JS data. - |&mut env, transform_result| env.to_js_value(&transform_result) + |&mut env, transform_result| env.to_js_value(&transform_result), ) } +/// Arguments passed to the JS custom read function. +struct ReadArgs { + file: PathBuf, + callback: Box napi::Result + Send>, +} + +/// Arguments passed to the JS custom resolve function. +struct ResolveArgs { + specifier: String, + originating_file: PathBuf, + callback: Box napi::Result + Send>, +} + +/// Buffer containing cached source inputs. This wrapper struct is only necessary to be +/// marked `Send` so it can be used with a `static` lifetime as required by +/// `ThreadsafeFunction`. Since `JsSourceProvider` uses this in an `Arc>` +/// This can outlive the `JsSourceProvider` if necessary. +struct InputCache { + inputs: Vec<*mut String>, +} + +unsafe impl Send for InputCache {} + +impl Drop for InputCache { + fn drop(&mut self) { + for ptr in self.inputs.iter() { + std::mem::drop(unsafe { Box::from_raw(*ptr) }); + } + } +} + +/// A `SourceProvider` implementation which uses JS implementations where given, falling +/// back to a wrapped `SourceProvider` where not given. +struct JsSourceProvider

+where + P: SourceProvider, +{ + /// Buffer with cached source files. + cache: Arc>, + + /// Fallback provider to use when no custom JS implementation is available for an + /// operation. + fallback_provider: P, + + /// Custom JS `read()` function. + maybe_read: Option>, + + /// Custom JS `resolve()` function. + maybe_resolve: Option>, +} + +impl JsSourceProvider

{ + /// Creates a new `JsSourceProvider`. + fn new( + fallback_provider: P, + maybe_read: Option>, + maybe_resolve: Option>, + ) -> JsSourceProvider

{ + JsSourceProvider { + cache: Arc::new(Mutex::new(InputCache { inputs: Vec::new() })), + fallback_provider, + maybe_read, + maybe_resolve, + } + } +} + +// JS implementations use thread-safe functions, so `JsSourceProvider` is thread-safe as +// long as the fallback provider is also thread-safe. +unsafe impl Send for JsSourceProvider

{} +unsafe impl Sync for JsSourceProvider

{} + +#[async_trait] +impl SourceProvider for JsSourceProvider

{ + async fn read<'a>(&'a self, file: &Path) -> std::io::Result<&'a str> { + let cache = self.cache.clone(); + match self.maybe_read.clone() { + // Use fallback provider if no JS `read()` implementation exists. + None => self.fallback_provider.read(file).await, + // Use JS `read()` implementation. + Some(read) => { + let file = file.to_owned(); + + // Wait until JS calls back with the result. + CallbackFuture::>::new(move |complete| { + // `complete` can only be called once, but JS could invoke this callback + // multiple times. Use a `Cell` to hide the mutability and restrict it to + // only be called once. + let complete = Cell::new(Some(complete)); + + // Invoke the JS `read()` function. + read.call(ReadArgs { + file, + // JS invokes this callback with the result. It follows Node async + // conventions (`cb(error, result)`). + callback: Box::new(move |ctx| { + // Get completion callback. This function can only be called once by JS + // so any subsequent executions should error immediately. + let complete = complete.take().ok_or(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "Callback invoked twice.", + ))?; + + // Validate no errors were thrown. + let error = ctx.get::(0)?; + let error_type = error.get_type()?; + if error_type != napi::ValueType::Null && error_type != napi::ValueType::Undefined { + complete(Err(std::io::Error::new(std::io::ErrorKind::Other, format!( + "`read()` threw error:\n{}", + error.coerce_to_string()?.into_utf8()?.as_str()?.to_owned(), + )))); + return ctx.env.get_undefined(); + } + + // Validate that result was a string. + let result = ctx.get::(1)?; + let result_type = result.get_type()?; + if result_type != napi::ValueType::String { + complete(Err(std::io::Error::new(std::io::ErrorKind::Other, format!( + "Expected `read()` to return a value of type `{}`, but it returned a value of type `{}` instead.", napi::ValueType::String, result_type, + )))); + return ctx.env.get_undefined(); + } + + // Get file contents and add to the source file cache. + let result = result.coerce_to_string()?.into_utf8()?.as_str()?.to_owned(); + let ptr = Box::into_raw(Box::new(result)); + cache.lock().unwrap().inputs.push(ptr); + + // SAFETY: this is safe because the pointer is not dropped until the + // `JsSourceProvider` is, and we never remove from the list of pointers + // stored in the vector. + complete(Ok(unsafe { &*ptr })); + + ctx.env.get_undefined() + }), + }, ThreadsafeFunctionCallMode::Blocking); + }).await + } + } + } + + async fn resolve(&self, specifier: &str, originating_file: &Path) -> Result> { + match self.maybe_resolve.clone() { + // Use fallback provider if no JS `resolve()` implementation exists. + None => self.fallback_provider.resolve(specifier, originating_file).await, + // Use JS `resolve()` implementation. + Some(resolve) => { + let specifier = specifier.to_owned(); + let originating_file = originating_file.to_path_buf().to_owned(); + + // Wait until JS calls back with the result. + CallbackFuture::>>::new(move |complete| { + // `complete` can only be called once, but JS could invoke this callback + // multiple times. Use a `Cell` to hide the mutability and restrict it to + // only be called once. + let complete = Cell::new(Some(complete)); + + // Invoke the JS `resolve()` function. + resolve.call(ResolveArgs { + specifier, + originating_file, + // JS invokes this callback with the result. + callback: Box::new(move |ctx| { + // Get completion callback. This function can only be called once by JS + // so any subsequent executions should error immediately. + let complete = complete.take().ok_or(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "Callback invoked twice.", + ))?; + + // Check for error. + let error = ctx.get::(0)?; + let error_type = error.get_type()?; + if error_type != napi::ValueType::Null && error_type != napi::ValueType::Undefined { + complete(Err(Error { + kind: BundleErrorKind::IOError(std::io::Error::new( + std::io::ErrorKind::Other, + format!("`resolve()` threw error:\n{}", error.coerce_to_string()?.into_utf8()?.as_str()?.to_owned()), + )), + loc: None, + })); + return ctx.env.get_undefined(); + } + + // Validate that result was a string. + let result = ctx.get::(1)?; + let result_type = result.get_type()?; + if result_type != napi::ValueType::String { + complete(Err(Error { + kind: BundleErrorKind::IOError(std::io::Error::new( + std::io::ErrorKind::Other, + format!("Expected `resolve()` to return a value of type `{}`, but it returned a value of type `{}` instead.", napi::ValueType::String, result_type), + )), + loc: None, + })); + return ctx.env.get_undefined(); + } + + // Convert JS string into a Rust `PathBuf`. + let resolved = result.coerce_to_string()?.into_utf8()?.as_str()?.to_owned(); + let resolved = Path::new(&resolved).to_path_buf(); + complete(Ok(resolved)); + + ctx.env.get_undefined() + }), + }, ThreadsafeFunctionCallMode::Blocking); + }).await + } + } + } +} + #[cfg(not(target_arch = "wasm32"))] #[module_exports] fn init(mut exports: JsObject) -> napi::Result<()> { @@ -344,8 +650,8 @@ fn compile<'i>(code: &'i str, config: &Config) -> Result( - fs: &'i FileProvider, +async fn compile_bundle<'i, P: SourceProvider>( + fs: &'i P, config: &BundleConfig, ) -> Result> { let mut source_map = if config.source_map.unwrap_or(false) { From 2fa7a0efd33696d009e9ed42b0f3d48fd9d563ba Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Sun, 5 Jun 2022 00:27:07 -0700 Subject: [PATCH 4/5] Add JS shim and types for `bundleAsync()` so the public API is just `(...args) => Promise` (#174) This hides the fact that `napi-rs` doesn't actually do anything with the return value of a threadsafe JS function, so any returned data would be dropped. Instead, Parcel adds a callback function which gets invoked by JS with the resulting value, following Node conventions of `callback(err, result)`. This is unergonimic as an API, so the JS shim exposes a `Promise`-based interface which gets converted to the callback behavior required by `napi-rs`. This also includes tests for all the common use cases and error conditions of custom JS resolvers. --- node/index.d.ts | 22 +++ node/index.js | 45 +++++ test-bundle.mjs | 262 +++++++++++++++++++++++++++++ tests/testdata/css/baz.css | 1 + tests/testdata/css/foo.css | 3 + tests/testdata/css/hello/world.css | 3 + 6 files changed, 336 insertions(+) create mode 100644 test-bundle.mjs create mode 100644 tests/testdata/css/baz.css create mode 100644 tests/testdata/css/foo.css create mode 100644 tests/testdata/css/hello/world.css diff --git a/node/index.d.ts b/node/index.d.ts index e1a43102..7dfbc9fa 100644 --- a/node/index.d.ts +++ b/node/index.d.ts @@ -197,3 +197,25 @@ export declare function browserslistToTargets(browserslist: string[]): Targets; * Bundles a CSS file and its dependencies, inlining @import rules. */ export declare function bundle(options: BundleOptions): TransformResult; + +export interface AsyncBundleOptions extends BundleOptions { + resolver?: Resolver; +} + +/** Custom resolver to use when loading CSS files. */ +export interface Resolver { + /** Read the given file and return its contents as a string. */ + read?: (file: string) => string | Promise; + + /** + * Resolve the given CSS import specifier from the provided originating file to a + * path which gets passed to `read()`. + */ + resolve?: (specifier: string, originatingFile: string) => string | Promise; +} + +/** + * Bundles a CSS file and its dependencies asynchronously, inlining @import rules. + * Also supports custom resolvers. + */ +export declare function bundleAsync(options: AsyncBundleOptions): Promise; diff --git a/node/index.js b/node/index.js index ba7fe4ee..508de4e9 100644 --- a/node/index.js +++ b/node/index.js @@ -23,3 +23,48 @@ if (process.env.CSS_TRANSFORMER_WASM) { } module.exports.browserslistToTargets = require('./browserslistToTargets'); + +// Include a small JS-shim for `bundleAsync()` to convert make `resolver` more ergonomic. +const {bundleAsync} = module.exports; +module.exports.bundleAsync = (opts, ...rest) => { + return bundleAsync({ + ...opts, + resolver: opts.resolver && { + ...opts.resolver, + read: opts.resolver.read && normalizeJsCallback(opts.resolver.read.bind(opts.resolver)), + resolve: opts.resolver.resolve && normalizeJsCallback(opts.resolver.resolve.bind(opts.resolver)), + }, + }, ...rest); +}; + +// `napi-rs` ignores JS function return values, so any results must be passed back to +// the Rust side via a callback rather than a returned value or `Promise`. This +// callback also follows NodeJS conventions (`callback(err, result)`). Managing the +// error and callback are annoying for users, so this converts a typical JS function +// which returns its result in a `Promise` into a N-API-compatible function which +// accepts and propagates its results to a callback. +function normalizeJsCallback(func) { + return (...args) => { + // Splice out `[...args, callback]`. + const funcArgs = args.slice(0, -1); + const callback = args[args.length - 1]; + + // Invoke the inner function, normalize to a `Promise`, and then invoke the callback + // function with Node conventions of `callback(err, result)`. + toPromise(() => func(...funcArgs)).then( + (result) => callback(null, result), + (err) => callback(err, null), + ); + }; +} + +// Converts the given function execution to return a `Promise` instead of returning or +// erroring synchronously. This is different from `Promise.resolve(func())` in that a +// synchronous `throw` statement is converted to a `Promise` rejection. +function toPromise(func) { + try { + return Promise.resolve(func()); + } catch (err) { + return Promise.reject(err); + } +} diff --git a/test-bundle.mjs b/test-bundle.mjs new file mode 100644 index 00000000..eb1ad310 --- /dev/null +++ b/test-bundle.mjs @@ -0,0 +1,262 @@ +import path from 'path'; +import css from './node/index.js'; + +(async function testResolver() { + const inMemoryFs = new Map(Object.entries({ + 'foo.css': ` +@import 'root:bar.css'; + +.foo { color: red; } + `.trim(), + + 'bar.css': ` +@import 'root:hello/world.css'; + +.bar { color: green; } + `.trim(), + + 'hello/world.css': ` +.baz { color: blue; } + `.trim(), + })); + + const { code: buffer } = await css.bundleAsync({ + filename: 'foo.css', + resolver: { + read(file) { + const result = inMemoryFs.get(path.normalize(file)); + if (!result) throw new Error(`Could not find ${file} in ${ + Array.from(inMemoryFs.keys()).join(', ')}.`); + return result; + }, + + resolve(specifier) { + return specifier.slice('root:'.length); + }, + }, + }); + const code = buffer.toString('utf-8').trim(); + + const expected = ` +.baz { + color: #00f; +} + +.bar { + color: green; +} + +.foo { + color: red; +} + `.trim(); + if (code !== expected) throw new Error(`\`testResolver()\` failed. Expected:\n${expected}\n\nGot:\n${code}`); +})(); + +(async function testOnlyCustomRead() { + const inMemoryFs = new Map(Object.entries({ + 'foo.css': ` +@import 'hello/world.css'; + +.foo { color: red; } + `.trim(), + + 'hello/world.css': ` +@import '../bar.css'; + +.bar { color: green; } + `.trim(), + + 'bar.css': ` +.baz { color: blue; } + `.trim(), + })); + + const { code: buffer } = await css.bundleAsync({ + filename: 'foo.css', + resolver: { + read(file) { + const result = inMemoryFs.get(path.normalize(file)); + if (!result) throw new Error(`Could not find ${file} in ${ + Array.from(inMemoryFs.keys()).join(', ')}.`); + return result; + }, + }, + }); + const code = buffer.toString('utf-8').trim(); + + const expected = ` +.baz { + color: #00f; +} + +.bar { + color: green; +} + +.foo { + color: red; +} + `.trim(); + if (code !== expected) throw new Error(`\`testOnlyCustomRead()\` failed. Expected:\n${expected}\n\nGot:\n${code}`); +})(); + +(async function testOnlyCustomResolve() { + const root = path.join('tests', 'testdata', 'css'); + const { code: buffer } = await css.bundleAsync({ + filename: path.join(root, 'foo.css'), + resolver: { + resolve(specifier) { + // Strip `root:` prefix off specifier and resolve it as an absolute path + // in the test data root. + return path.join(root, specifier.slice('root:'.length)); + }, + }, + }); + const code = buffer.toString('utf-8').trim(); + + const expected = ` +.baz { + color: #00f; +} + +.bar { + color: green; +} + +.foo { + color: red; +} + `.trim(); + if (code !== expected) throw new Error(`\`testOnlyCustomResolve()\` failed. Expected:\n${expected}\n\nGot:\n${code}`); +})(); + +(async function testReadThrow() { + let error = undefined; + try { + await css.bundleAsync({ + filename: 'foo.css', + resolver: { + read(file) { + throw new Error(`Oh noes! Failed to read \`${file}\`.`); + } + }, + }); + } catch (err) { + error = err; + } + + if (!error) throw new Error(`\`testReadThrow()\` failed. Expected \`bundleAsync()\` to throw, but it did not.`); + if (!error.message.includes(`\`read()\` threw error:`) || !error.message.includes(`Oh noes! Failed to read \`foo.css\`.`)) { + throw new Error(`\`testReadThrow()\` failed. Expected \`bundleAsync()\` to throw a specific error message, but it threw a different error:\n${error.message}`); + } +})(); + +(async function testResolveThrow() { + let error = undefined; + try { + await css.bundleAsync({ + filename: 'tests/testdata/css/foo.css', + resolver: { + resolve(specifier, originatingFile) { + throw new Error(`Oh noes! Failed to resolve \`${specifier}\` from \`${ + originatingFile}\`.`); + } + }, + }); + } catch (err) { + error = err; + } + + if (!error) throw new Error(`\`testResolveThrow()\` failed. Expected \`bundleAsync()\` to throw, but it did not.`); + if (!error.message.includes(`\`resolve()\` threw error:`) || !error.message.includes(`Oh noes! Failed to resolve \`root:hello/world.css\` from \`tests/testdata/css/foo.css\`.`)) { + throw new Error(`\`testResolveThrow()\` failed. Expected \`bundleAsync()\` to throw a specific error message, but it threw a different error:\n${error.message}`); + } +})(); + +(async function testReadReturnNonString() { + let error = undefined; + try { + await css.bundleAsync({ + filename: 'foo.css', + resolver: { + read() { + return 1234; // Returns a non-string value. + } + }, + }); + } catch (err) { + error = err; + } + + if (!error) throw new Error(`\`testReadReturnNonString()\` failed. Expected \`bundleAsync()\` to throw, but it did not.`); + if (!error.message.includes(`Expected \`read()\` to return a value of type \`String\`, but it returned a value of type \`Number\` instead.`)) { + throw new Error(`\`testReadReturnNonString()\` failed. Expected \`bundleAsync()\` to throw a specific error message, but it threw a different error:\n${error.message}`); + } +})(); + +(async function testResolveReturnNonString() { + let error = undefined; + try { + await css.bundleAsync({ + filename: 'tests/testdata/css/foo.css', + resolver: { + resolve() { + return 1234; // Returns a non-string value. + } + }, + }); + } catch (err) { + error = err; + } + + if (!error) throw new Error(`\`testResolveReturnNonString()\` failed. Expected \`bundleAsync()\` to throw, but it did not.`); + if (!error.message.includes(`Expected \`resolve()\` to return a value of type \`String\`, but it returned a value of type \`Number\` instead.`)) { + throw new Error(`\`testResolveReturnNonString()\` failed. Expected \`bundleAsync()\` to throw a specific error message, but it threw a different error:\n${error.message}`); + } +})(); + +(async function testThis() { + const inMemoryFs = new Map(Object.entries({ + 'foo.css': ` +@import './bar.css'; + +.foo { color: red; } + `.trim(), + + 'bar.css': ` +@import './hello/world.css'; + +.bar { color: green; } + `.trim(), + + 'hello/world.css': ` +.baz { color: blue; } + `.trim(), + })); + + let readThis = undefined; + let resolveThis = undefined; + const resolver = { + read: function (file) { + readThis = this; + const result = inMemoryFs.get(path.normalize(file)); + if (!result) throw new Error(`Could not find ${file} in ${ + Array.from(inMemoryFs.keys()).join(', ')}.`); + return result; + }, + resolve: function (specifier, originatingFile) { + resolveThis = this; + return path.join(originatingFile, '..', specifier); + }, + }; + await css.bundleAsync({ + filename: 'foo.css', + resolver, + }); + + if (readThis !== resolver) throw new Error(`\`testThis()\` failed. Expected \`read()\` to be called with the \`resolver\` as \`this\`, but instead it was called with:\n${readThis}`); + if (resolveThis !== resolver) throw new Error(`\`testThis()\` failed. Expected \`resolve()\` to be called with the \`resolver\` as \`this\`, but instead it was called with:\n${resolveThis}`); +})(); + +console.log('PASSED!'); diff --git a/tests/testdata/css/baz.css b/tests/testdata/css/baz.css new file mode 100644 index 00000000..ccb3274e --- /dev/null +++ b/tests/testdata/css/baz.css @@ -0,0 +1 @@ +.baz { color: blue; } diff --git a/tests/testdata/css/foo.css b/tests/testdata/css/foo.css new file mode 100644 index 00000000..95099612 --- /dev/null +++ b/tests/testdata/css/foo.css @@ -0,0 +1,3 @@ +@import 'root:hello/world.css'; + +.foo { color: red; } diff --git a/tests/testdata/css/hello/world.css b/tests/testdata/css/hello/world.css new file mode 100644 index 00000000..bc1763bb --- /dev/null +++ b/tests/testdata/css/hello/world.css @@ -0,0 +1,3 @@ +@import 'root:baz.css'; + +.bar { color: green; } From 7140db9d896a76eaeb5058e303eca41e9725840c Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Sun, 5 Jun 2022 00:27:07 -0700 Subject: [PATCH 5/5] Add unsupported resolver error to `bundle()` (#174) This rejects immediately if the user attempts to pass a `resolver` property to the synchronous `bundle()` function. Since communciating across between background threads and the main thread is quite tricky without an asynchronous hook, `bundle()` does not support custom resolvers. --- node/src/lib.rs | 12 +++++++++++- test-bundle.mjs | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/node/src/lib.rs b/node/src/lib.rs index d6b321dc..c2e1e051 100644 --- a/node/src/lib.rs +++ b/node/src/lib.rs @@ -138,7 +138,17 @@ fn transform_style_attribute(ctx: CallContext) -> napi::Result { #[js_function(1)] fn bundle(ctx: CallContext) -> napi::Result { let opts = ctx.get::(0)?; - let config: BundleConfig = ctx.env.from_js_value(opts)?; + let config: BundleConfig = ctx.env.from_js_value(&opts)?; + + // Throw early error if user mistakenly passes `resolver` into synchronous `bundle()`. + let resolver = opts.get::<&str, JsUnknown>("resolver")?; + if resolver.is_some() { + return Err(napi::Error::new( + napi::Status::InvalidArg, + "`bundle()` doesn't support custom JS resolvers but received a `resolver` property. Use `bundleAsync()` instead.".to_owned()), + ); + } + let fs = FileProvider::new(); let async_runtime = Runtime::new()?; let res = async_runtime.block_on(compile_bundle(&fs, &config)); diff --git a/test-bundle.mjs b/test-bundle.mjs index eb1ad310..c0d578b0 100644 --- a/test-bundle.mjs +++ b/test-bundle.mjs @@ -1,6 +1,23 @@ import path from 'path'; import css from './node/index.js'; +(function testBundleRejectsResolver() { + let error = undefined; + try { + css.bundle({ + filename: 'foo.css', + resolver: {}, + }); + } catch (err) { + error = err; + } + + if (!error) throw new Error(`\`testBundleRejectsResolver()\` failed. Expected \`bundle()\` to throw, but it did not.`); + if (!error.message.includes(`\`bundle()\` doesn't support custom JS resolvers`) || !error.message.includes(`Use \`bundleAsync()\` instead.`)) { + throw new Error(`\`testBundleRejectsResolver()\` failed. Expected \`bundle()\` to throw a specific error message, but it threw a different error:\n${error.message}`); + } +}()); + (async function testResolver() { const inMemoryFs = new Map(Object.entries({ 'foo.css': `