From f6128f2b8f415a061752b328fa4aaab01536ed26 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Thu, 19 Dec 2024 10:13:34 +1100 Subject: [PATCH 1/3] [V3] Add support for passed in targets --- .../atlaspack/src/requests/target_request.rs | 118 ++++++++++++++++-- .../src/types/atlaspack_options.rs | 2 + 2 files changed, 108 insertions(+), 12 deletions(-) diff --git a/crates/atlaspack/src/requests/target_request.rs b/crates/atlaspack/src/requests/target_request.rs index 9672fb753..513f20962 100644 --- a/crates/atlaspack/src/requests/target_request.rs +++ b/crates/atlaspack/src/requests/target_request.rs @@ -237,7 +237,7 @@ impl TargetRequest { fn load_config( &self, - request_context: RunRequestContext, + request_context: &RunRequestContext, ) -> Result, anyhow::Error> { // TODO Invalidations let mut config = match request_context.config().load_package_json::() { @@ -316,7 +316,7 @@ impl TargetRequest { &self, request_context: RunRequestContext, ) -> Result>, anyhow::Error> { - let config = self.load_config(request_context)?; + let config = self.load_config(&request_context)?; let mut targets: Vec> = Vec::new(); let builtin_targets = [ @@ -365,6 +365,19 @@ impl TargetRequest { .map(|(name, descriptor)| CustomTarget { descriptor, name }); for custom_target in custom_targets { + if request_context + .options + .targets + .as_ref() + .is_some_and(|targets| !targets.contains(&String::from(custom_target.name))) + { + tracing::debug!( + "Skipping custom target {} as it doesn't match passed in target options", + custom_target.name + ); + continue; + } + let mut dist = None; if let Some(value) = config.contents.fields.get(custom_target.name) { match value { @@ -695,7 +708,7 @@ mod tests { use regex::Regex; - use atlaspack_core::types::version::Version; + use atlaspack_core::types::{version::Version, AtlaspackOptions}; use atlaspack_filesystem::in_memory_file_system::InMemoryFileSystem; use crate::test_utils::{request_tracker, RequestTrackerTestOptions}; @@ -729,6 +742,7 @@ mod tests { async fn targets_from_config( package_json: String, browserslistrc: Option, + atlaspack_options: Option, ) -> Result { let fs = InMemoryFileSystem::default(); let project_root = PathBuf::default(); @@ -754,6 +768,7 @@ mod tests { search_path: project_root.join(&package_dir), project_root, fs: Arc::new(fs), + atlaspack_options: atlaspack_options.unwrap_or_default(), ..Default::default() }) .run_request(request) @@ -771,6 +786,7 @@ mod tests { let targets = targets_from_config( format!(r#"{{ "targets": {{ "{builtin_target}": true }} }}"#,), None, + None, ) .await; @@ -789,6 +805,7 @@ mod tests { let targets = targets_from_config( format!(r#"{{ "{}": "dist/main.rs" }}"#, builtin_target), None, + None, ) .await; @@ -815,6 +832,7 @@ mod tests { }}"# ), None, + None, ) .await; @@ -851,6 +869,7 @@ mod tests { ), ), None, + None, ) .await; @@ -880,7 +899,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_error_when_scope_hoisting_disabled_for_library_targets() { let assert_error = move |name, package_json| async move { - let targets = targets_from_config(package_json, None).await; + let targets = targets_from_config(package_json, None, None).await; assert_eq!( targets.map_err(to_deterministic_error), @@ -962,6 +981,7 @@ mod tests { let targets = targets_from_config( format!(r#"{{ "targets": {{ "{builtin_target}": false }} }}"#), None, + None, ) .await; @@ -977,7 +997,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_default_target_when_no_targets_are_specified() { - let targets = targets_from_config(String::from("{}"), None).await; + let targets = targets_from_config(String::from("{}"), None, None).await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1000,8 +1020,12 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_default_builtin_browser_target() { - let targets = - targets_from_config(String::from(r#"{ "browser": "build/browser.js" }"#), None).await; + let targets = targets_from_config( + String::from(r#"{ "browser": "build/browser.js" }"#), + None, + None, + ) + .await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1042,6 +1066,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1070,7 +1095,8 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_default_builtin_main_target() { - let targets = targets_from_config(String::from(r#"{ "main": "./build/main.js" }"#), None).await; + let targets = + targets_from_config(String::from(r#"{ "main": "./build/main.js" }"#), None, None).await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1107,6 +1133,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1132,7 +1159,8 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_default_builtin_module_target() { - let targets = targets_from_config(String::from(r#"{ "module": "module.js" }"#), None).await; + let targets = + targets_from_config(String::from(r#"{ "module": "module.js" }"#), None, None).await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1169,6 +1197,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1208,6 +1237,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1232,7 +1262,8 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_default_builtin_types_target() { - let targets = targets_from_config(String::from(r#"{ "types": "./types.d.ts" }"#), None).await; + let targets = + targets_from_config(String::from(r#"{ "types": "./types.d.ts" }"#), None, None).await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1268,6 +1299,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1337,8 +1369,12 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn returns_custom_targets_with_defaults() { - let targets = - targets_from_config(String::from(r#"{ "targets": { "custom": {} } } "#), None).await; + let targets = targets_from_config( + String::from(r#"{ "targets": { "custom": {} } } "#), + None, + None, + ) + .await; assert_eq!( targets.map_err(|e| e.to_string()), @@ -1366,6 +1402,58 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn returns_only_requested_custom_targets() { + let targets = targets_from_config( + String::from( + r#" + { + "targets": { + "custom-one": { + "context": "node", + "includeNodeModules": true, + "outputFormat": "commonjs" + }, + "custom-two": { + "context": "browser", + "outputFormat": "esmodule" + } + } + } + "#, + ), + None, + Some(AtlaspackOptions { + targets: Some(vec!["custom-two".into()]), + ..Default::default() + }), + ) + .await; + + assert_eq!( + targets.map_err(|e| e.to_string()), + Ok(RequestResult::Target(TargetRequestOutput { + entry: PathBuf::default(), + targets: vec![Target { + dist_dir: package_dir().join("dist/custom-two"), + dist_entry: None, + env: Arc::new(Environment { + context: EnvironmentContext::Browser, + output_format: OutputFormat::EsModule, + should_optimize: true, + engines: Engines { + browsers: Some(EnginesBrowsers::default()), + ..Engines::default() + }, + ..Environment::default() + }), + name: String::from("custom-two"), + ..Target::default() + }] + })) + ); + } + #[tokio::test(flavor = "multi_thread")] async fn returns_custom_targets() { let targets = targets_from_config( @@ -1384,6 +1472,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1424,6 +1513,7 @@ mod tests { "#, ), None, + None, ) .await; @@ -1474,6 +1564,7 @@ mod tests { firefox > 1 "#, )), + None, ) .await; @@ -1542,6 +1633,7 @@ mod tests { "#, ), None, + None, ) .await, Engines { @@ -1563,6 +1655,7 @@ mod tests { "#, ), None, + None, ) .await, Engines { @@ -1595,6 +1688,7 @@ mod tests { ), ), None, + None, ) .await; diff --git a/crates/atlaspack_core/src/types/atlaspack_options.rs b/crates/atlaspack_core/src/types/atlaspack_options.rs index 0f77ef111..e95e3e10c 100644 --- a/crates/atlaspack_core/src/types/atlaspack_options.rs +++ b/crates/atlaspack_core/src/types/atlaspack_options.rs @@ -36,6 +36,8 @@ pub struct AtlaspackOptions { pub mode: BuildMode, pub threads: Option, + + pub targets: Option>, } #[derive(Clone, Debug, Default, Hash, PartialEq, Serialize)] From b04ac0447b0041f47d1aa8f639e6d64660f7b442 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Thu, 19 Dec 2024 10:39:49 +1100 Subject: [PATCH 2/3] Improve target matching --- crates/atlaspack/src/requests/target_request.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/atlaspack/src/requests/target_request.rs b/crates/atlaspack/src/requests/target_request.rs index 513f20962..01ea19df0 100644 --- a/crates/atlaspack/src/requests/target_request.rs +++ b/crates/atlaspack/src/requests/target_request.rs @@ -369,7 +369,11 @@ impl TargetRequest { .options .targets .as_ref() - .is_some_and(|targets| !targets.contains(&String::from(custom_target.name))) + .is_some_and(|targets| { + !targets + .iter() + .any(|target_name| target_name == custom_target.name) + }) { tracing::debug!( "Skipping custom target {} as it doesn't match passed in target options", From 3cbb1037d24124386ffc289f8f982ec877c2cd22 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Thu, 19 Dec 2024 14:56:06 +1100 Subject: [PATCH 3/3] Add support for custom targets from CLI --- .../atlaspack/src/requests/target_request.rs | 92 +++++++++++++----- .../requests/target_request/package_json.rs | 36 +------ .../src/types/atlaspack_options.rs | 44 ++++++++- .../core/integration-tests/test/javascript.js | 93 ++++++++----------- 4 files changed, 153 insertions(+), 112 deletions(-) diff --git a/crates/atlaspack/src/requests/target_request.rs b/crates/atlaspack/src/requests/target_request.rs index 01ea19df0..0beb343ea 100644 --- a/crates/atlaspack/src/requests/target_request.rs +++ b/crates/atlaspack/src/requests/target_request.rs @@ -19,18 +19,19 @@ use atlaspack_core::types::Environment; use atlaspack_core::types::EnvironmentContext; use atlaspack_core::types::ErrorKind; use atlaspack_core::types::OutputFormat; +use atlaspack_core::types::SourceField; +use atlaspack_core::types::SourceMapField; use atlaspack_core::types::SourceType; use atlaspack_core::types::Target; +use atlaspack_core::types::TargetDescriptor; use atlaspack_core::types::TargetSourceMapOptions; +use atlaspack_core::types::Targets; use atlaspack_resolver::IncludeNodeModules; use package_json::BrowserField; use package_json::BrowsersList; use package_json::BuiltInTargetDescriptor; use package_json::ModuleFormat; use package_json::PackageJson; -use package_json::SourceField; -use package_json::SourceMapField; -use package_json::TargetDescriptor; use crate::request_tracker::Request; use crate::request_tracker::ResultAndInvalidations; @@ -339,6 +340,24 @@ impl TargetRequest { ), ]; + let mut target_filter = None; + + if let Some(target_options) = &request_context.options.targets { + match target_options { + Targets::Filter(target_list) => { + target_filter = Some(target_list); + } + Targets::CustomTarget(custom_targets) => { + for (name, descriptor) in custom_targets.iter() { + targets.push(self.target_from_descriptor(None, &config, descriptor.clone(), name)?); + } + + // Custom targets have been passed in so let's just use those + return Ok(targets); + } + } + } + for builtin_target in builtin_targets { if builtin_target.dist.is_none() { continue; @@ -362,26 +381,14 @@ impl TargetRequest { .targets .custom_targets .iter() - .map(|(name, descriptor)| CustomTarget { descriptor, name }); + .map(|(name, descriptor)| CustomTarget { descriptor, name }) + .filter(|CustomTarget { name, .. }| { + target_filter + .as_ref() + .is_none_or(|targets| targets.iter().any(|target_name| target_name == name)) + }); for custom_target in custom_targets { - if request_context - .options - .targets - .as_ref() - .is_some_and(|targets| { - !targets - .iter() - .any(|target_name| target_name == custom_target.name) - }) - { - tracing::debug!( - "Skipping custom target {} as it doesn't match passed in target options", - custom_target.name - ); - continue; - } - let mut dist = None; if let Some(value) = config.contents.fields.get(custom_target.name) { match value { @@ -1406,6 +1413,47 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn returns_custom_targets_from_options_with_defaults() { + let targets = targets_from_config( + String::from(r#"{}"#), + None, + Some(AtlaspackOptions { + targets: Some(Targets::CustomTarget(BTreeMap::from([( + "custom".into(), + TargetDescriptor::default(), + )]))), + ..Default::default() + }), + ) + .await; + + assert_eq!( + targets.map_err(|e| e.to_string()), + Ok(RequestResult::Target(TargetRequestOutput { + entry: PathBuf::default(), + targets: vec![Target { + dist_dir: package_dir().join("dist").join("custom"), + dist_entry: None, + env: Arc::new(Environment { + context: EnvironmentContext::Browser, + engines: Engines { + browsers: Some(EnginesBrowsers::default()), + ..Engines::default() + }, + is_library: false, + output_format: OutputFormat::Global, + should_optimize: true, + should_scope_hoist: false, + ..Environment::default() + }), + name: String::from("custom"), + ..Target::default() + }] + })) + ); + } + #[tokio::test(flavor = "multi_thread")] async fn returns_only_requested_custom_targets() { let targets = targets_from_config( @@ -1428,7 +1476,7 @@ mod tests { ), None, Some(AtlaspackOptions { - targets: Some(vec!["custom-two".into()]), + targets: Some(Targets::Filter(vec!["custom-two".into()])), ..Default::default() }), ) diff --git a/crates/atlaspack/src/requests/target_request/package_json.rs b/crates/atlaspack/src/requests/target_request/package_json.rs index 7c9d5efa4..772303f0b 100644 --- a/crates/atlaspack/src/requests/target_request/package_json.rs +++ b/crates/atlaspack/src/requests/target_request/package_json.rs @@ -4,10 +4,8 @@ use std::fmt::Display; use std::path::{Path, PathBuf}; use atlaspack_core::types::engines::Engines; -use atlaspack_core::types::EnvironmentContext; use atlaspack_core::types::OutputFormat; -use atlaspack_core::types::TargetSourceMapOptions; -use atlaspack_resolver::IncludeNodeModules; +use atlaspack_core::types::TargetDescriptor; use serde::Deserialize; use serde::Deserializer; use serde_json::Value; @@ -35,23 +33,6 @@ pub enum BuiltInTargetDescriptor { TargetDescriptor(TargetDescriptor), } -#[derive(Debug, Clone, Default, Deserialize, PartialEq)] -#[serde(default, rename_all = "camelCase")] -pub struct TargetDescriptor { - pub context: Option, - pub dist_dir: Option, - pub dist_entry: Option, - pub engines: Option, - pub include_node_modules: Option, - pub is_library: Option, - pub optimize: Option, - pub output_format: Option, - pub public_url: Option, - pub scope_hoist: Option, - pub source: Option, - pub source_map: Option, -} - #[derive(Debug, Deserialize, PartialEq)] #[serde(rename_all = "lowercase")] pub enum ModuleFormat { @@ -99,21 +80,6 @@ pub struct PackageJson { pub fields: HashMap, } -#[derive(Debug, Clone, Deserialize, PartialEq)] -pub enum SourceField { - #[allow(unused)] - Source(String), - #[allow(unused)] - Sources(Vec), -} - -#[derive(Debug, Clone, Deserialize, PartialEq)] -#[serde(untagged)] -pub enum SourceMapField { - Bool(bool), - Options(TargetSourceMapOptions), -} - fn browser_field<'de, D>(deserializer: D) -> Result, D::Error> where D: Deserializer<'de>, diff --git a/crates/atlaspack_core/src/types/atlaspack_options.rs b/crates/atlaspack_core/src/types/atlaspack_options.rs index e95e3e10c..b0377c157 100644 --- a/crates/atlaspack_core/src/types/atlaspack_options.rs +++ b/crates/atlaspack_core/src/types/atlaspack_options.rs @@ -7,7 +7,10 @@ use serde::Deserializer; use serde::Serialize; use super::engines::Engines; +use super::EnvironmentContext; +use super::IncludeNodeModules; use super::OutputFormat; +use super::TargetSourceMapOptions; /// The options passed into Atlaspack either through the CLI or the programmatic API #[derive(Clone, Debug, Default, Deserialize, Serialize)] @@ -37,7 +40,46 @@ pub struct AtlaspackOptions { pub threads: Option, - pub targets: Option>, + pub targets: Option, +} + +#[derive(Clone, Debug, Hash, PartialEq, Deserialize, Serialize)] +#[serde(untagged)] +pub enum Targets { + Filter(Vec), + CustomTarget(BTreeMap), +} + +#[derive(Debug, Clone, Deserialize, Hash, PartialEq, Serialize)] +pub enum SourceField { + #[allow(unused)] + Source(String), + #[allow(unused)] + Sources(Vec), +} + +#[derive(Debug, Clone, Deserialize, Hash, PartialEq, Serialize)] +#[serde(untagged)] +pub enum SourceMapField { + Bool(bool), + Options(TargetSourceMapOptions), +} + +#[derive(Debug, Clone, Default, Deserialize, Hash, PartialEq, Serialize)] +#[serde(default, rename_all = "camelCase")] +pub struct TargetDescriptor { + pub context: Option, + pub dist_dir: Option, + pub dist_entry: Option, + pub engines: Option, + pub include_node_modules: Option, + pub is_library: Option, + pub optimize: Option, + pub output_format: Option, + pub public_url: Option, + pub scope_hoist: Option, + pub source: Option, + pub source_map: Option, } #[derive(Clone, Debug, Default, Hash, PartialEq, Serialize)] diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index d86f77f79..9eeef7e1d 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -1871,7 +1871,7 @@ describe('javascript', function () { assert.equal(output(), true); }); - it.v2('should not touch process.browser for target node', async function () { + it('should not touch process.browser for target node', async function () { let b = await bundle( path.join(__dirname, '/integration/process/index.js'), { @@ -1889,26 +1889,23 @@ describe('javascript', function () { assert.equal(output(), false); }); - it.v2( - 'should not touch process.browser for target electron-main', - async function () { - let b = await bundle( - path.join(__dirname, '/integration/process/index.js'), - { - targets: { - main: { - context: 'electron-main', - distDir: path.join(__dirname, '/integration/process/dist.js'), - }, + it('should not touch process.browser for target electron-main', async function () { + let b = await bundle( + path.join(__dirname, '/integration/process/index.js'), + { + targets: { + main: { + context: 'electron-main', + distDir: path.join(__dirname, '/integration/process/dist.js'), }, }, - ); + }, + ); - let output = await run(b); - assert.ok(output.toString().indexOf('process.browser') !== -1); - assert.equal(output(), false); - }, - ); + let output = await run(b); + assert.ok(output.toString().indexOf('process.browser') !== -1); + assert.equal(output(), false); + }); it('should replace process.browser for target electron-renderer', async function () { let b = await bundle( @@ -2000,25 +1997,19 @@ describe('javascript', function () { }, ); - it.v2( - 'should not exclude resolving specifiers that map to false in the browser field in node builds', - async () => { - let b = await bundle( - path.join( - __dirname, - '/integration/resolve-entries/pkg-ignore-browser/index.js', - ), - { - targets: ['node'], - }, - ); + it('should not exclude resolving specifiers that map to false in the browser field in node builds', async () => { + let b = await bundle( + path.join( + __dirname, + '/integration/resolve-entries/pkg-ignore-browser/index.js', + ), + { + targets: ['node'], + }, + ); - assert.equal( - await run(b), - 'this should only exist in non-browser builds', - ); - }, - ); + assert.equal(await run(b), 'this should only exist in non-browser builds'); + }); it.skip('should not resolve the browser field for --target=node', async function () { let b = await bundle( @@ -3458,26 +3449,20 @@ describe('javascript', function () { assert.deepEqual(res, {a: 4}); }); - // Enable this for v3 once hmr options is supported in the js_transformer - - it.v2( - 'should not use arrow functions for reexport declarations unless supported', - async function () { - // TODO: Fails in v3 due to "ENOENT: no such file or directory" for the - let b = await bundle( - path.join(__dirname, 'integration/js-export-arrow-support/index.js'), - { - // Remove comments containing "=>" - defaultTargetOptions: { - shouldOptimize: true, - }, + it('should not use arrow functions for reexport declarations unless supported', async function () { + let b = await bundle( + path.join(__dirname, 'integration/js-export-arrow-support/index.js'), + { + // Remove comments containing "=>" + defaultTargetOptions: { + shouldOptimize: true, }, - ); + }, + ); - let content = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8'); - assert(!content.includes('=>')); - }, - ); + let content = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8'); + assert(!content.includes('=>')); + }); it('should support classes that extend from another using default browsers', async () => { await fsFixture(overlayFS, __dirname)`