Skip to content

Fix --args and handle merging in @server and @client #4212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
7 changes: 5 additions & 2 deletions packages/cli/src/build/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ impl AppBuilder {
}
}

#[allow(clippy::too_many_arguments)]
pub(crate) async fn open(
&mut self,
devserver_ip: SocketAddr,
Expand All @@ -425,6 +426,7 @@ impl AppBuilder {
open_browser: bool,
always_on_top: bool,
build_id: BuildId,
args: &[String],
) -> Result<()> {
let krate = &self.build;

Expand Down Expand Up @@ -501,7 +503,7 @@ impl AppBuilder {
| Platform::MacOS
| Platform::Windows
| Platform::Linux
| Platform::Liveview => self.open_with_main_exe(envs)?,
| Platform::Liveview => self.open_with_main_exe(envs, args)?,
};

self.builds_opened += 1;
Expand Down Expand Up @@ -732,12 +734,13 @@ impl AppBuilder {
/// paths right now, but they will when we start to enable things like swift integration.
///
/// Server/liveview/desktop are all basically the same, though
fn open_with_main_exe(&mut self, envs: Vec<(&str, String)>) -> Result<()> {
fn open_with_main_exe(&mut self, envs: Vec<(&str, String)>, args: &[String]) -> Result<()> {
let main_exe = self.app_exe();

tracing::debug!("Opening app with main exe: {main_exe:?}");

let mut child = Command::new(main_exe)
.args(args)
.envs(envs)
.env_remove("CARGO_MANIFEST_DIR") // running under `dx` shouldn't expose cargo-only :
.stderr(Stdio::piped())
Expand Down
16 changes: 13 additions & 3 deletions packages/cli/src/build/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ pub(crate) struct BuildRequest {
pub(crate) triple: Triple,
pub(crate) device: bool,
pub(crate) package: String,
pub(crate) main_target: String,
pub(crate) features: Vec<String>,
pub(crate) extra_cargo_args: Vec<String>,
pub(crate) extra_rustc_args: Vec<String>,
Expand Down Expand Up @@ -463,7 +464,11 @@ impl BuildRequest {
///
/// Note: Build requests are typically created only when the CLI is invoked or when significant
/// changes are detected in the `Cargo.toml` (e.g., features added or removed).
pub(crate) async fn new(args: &TargetArgs, workspace: Arc<Workspace>) -> Result<Self> {
pub(crate) async fn new(
args: &TargetArgs,
main_target: Option<String>,
workspace: Arc<Workspace>,
) -> Result<Self> {
let crate_package = workspace.find_main_package(args.package.clone())?;

let config = workspace
Expand Down Expand Up @@ -506,6 +511,10 @@ impl BuildRequest {
})
.unwrap_or(workspace.krates[crate_package].name.clone());

// Use the main_target for the client + server build if it is set, otherwise use the target name for this
// specific build
let main_target = main_target.unwrap_or(target_name.clone());

let crate_target = main_package
.targets
.iter()
Expand Down Expand Up @@ -727,6 +736,7 @@ impl BuildRequest {
extra_cargo_args,
release,
package,
main_target,
skip_assets: args.skip_assets,
base_path: args.base_path.clone(),
wasm_split: args.wasm_split,
Expand Down Expand Up @@ -2737,7 +2747,7 @@ impl BuildRequest {
/// target/dx/build/app/web/server.exe
pub(crate) fn build_dir(&self, platform: Platform, release: bool) -> PathBuf {
self.internal_out_dir()
.join(self.executable_name())
.join(&self.main_target)
.join(if release { "release" } else { "debug" })
.join(platform.build_folder_name())
}
Expand All @@ -2748,7 +2758,7 @@ impl BuildRequest {
/// target/dx/bundle/app/public/
pub(crate) fn bundle_dir(&self, platform: Platform) -> PathBuf {
self.internal_out_dir()
.join(self.executable_name())
.join(&self.main_target)
.join("bundle")
.join(platform.build_folder_name())
}
Expand Down
173 changes: 57 additions & 116 deletions packages/cli/src/cli/build.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::{cli::*, AppBuilder, BuildRequest, Workspace, PROFILE_SERVER};
use crate::{cli::*, AppBuilder, BuildRequest, Workspace};
use crate::{BuildMode, Platform};
use target_lexicon::Triple;

use super::target::{TargetArgs, TargetCmd};
use super::target::TargetArgs;

/// Build the Rust Dioxus app and all of its assets.
///
Expand All @@ -16,42 +15,9 @@ pub struct BuildArgs {
#[clap(long)]
pub(crate) fullstack: Option<bool>,

/// The feature to use for the client in a fullstack app [default: "web"]
#[clap(long)]
pub(crate) client_features: Vec<String>,

/// The feature to use for the server in a fullstack app [default: "server"]
#[clap(long)]
pub(crate) server_features: Vec<String>,

/// Build with custom profile for the fullstack server
#[clap(long, default_value_t = PROFILE_SERVER.to_string())]
pub(crate) server_profile: String,

/// The target to build for the server.
///
/// This can be different than the host allowing cross-compilation of the server. This is useful for
/// platforms like Cloudflare Workers where the server is compiled to wasm and then uploaded to the edge.
#[clap(long)]
pub(crate) server_target: Option<Triple>,

/// Arguments for the build itself
#[clap(flatten)]
pub(crate) build_arguments: TargetArgs,

/// A list of additional targets to build.
///
/// Server and Client are special targets that integrate with `dx serve`, while `crate` is a generic.
///
/// ```sh
/// dx serve \
/// client --target aarch64-apple-darwin \
/// server --target wasm32-unknown-unknown \
/// crate --target aarch64-unknown-linux-gnu --package foo \
/// crate --target x86_64-unknown-linux-gnu --package bar
/// ```
#[command(subcommand)]
pub(crate) targets: Option<TargetCmd>,
}

pub struct BuildTargets {
Expand All @@ -60,6 +26,40 @@ pub struct BuildTargets {
}

impl BuildArgs {
fn default_client(&self) -> &TargetArgs {
&self.build_arguments
}

fn default_server(&self, client: &BuildRequest) -> Option<&TargetArgs> {
// Now resolve the builds that we need to.
// These come from the args, but we'd like them to come from the `TargetCmd` chained object
//
// The process here is as follows:
//
// - Create the BuildRequest for the primary target
// - If that BuildRequest is "fullstack", then add the client features
// - If that BuildRequest is "fullstack", then also create a BuildRequest for the server
// with the server features
//
// This involves modifying the BuildRequest to add the client features and server features
// only if we can properly detect that it's a fullstack build. Careful with this, since
// we didn't build BuildRequest to be generally mutable.
let default_server = client.enabled_platforms.contains(&Platform::Server);

// Make sure we set the fullstack platform so we actually build the fullstack variant
// Users need to enable "fullstack" in their default feature set.
// todo(jon): fullstack *could* be a feature of the app, but right now we're assuming it's always enabled
//
// Now we need to resolve the client features
let fullstack = ((default_server || client.fullstack_feature_enabled())
|| self.fullstack.unwrap_or(false))
&& self.fullstack != Some(false);

fullstack.then_some(&self.build_arguments)
}
}

impl CommandWithPlatformOverrides<BuildArgs> {
pub async fn build(self) -> Result<StructuredOutput> {
tracing::info!("Building project...");

Expand Down Expand Up @@ -92,89 +92,30 @@ impl BuildArgs {
// do some logging to ensure dx matches the dioxus version since we're not always API compatible
workspace.check_dioxus_version_against_cli();

let mut server = None;
let client_args = match &self.client {
Some(client) => &client.build_arguments,
None => self.shared.default_client(),
};
let client = BuildRequest::new(client_args, None, workspace.clone()).await?;

let client = match self.targets {
// A simple `dx serve` command with no explicit targets
None => {
// Now resolve the builds that we need to.
// These come from the args, but we'd like them to come from the `TargetCmd` chained object
//
// The process here is as follows:
//
// - Create the BuildRequest for the primary target
// - If that BuildRequest is "fullstack", then add the client features
// - If that BuildRequest is "fullstack", then also create a BuildRequest for the server
// with the server features
//
// This involves modifying the BuildRequest to add the client features and server features
// only if we can properly detect that it's a fullstack build. Careful with this, since
// we didn't build BuildRequest to be generally mutable.
let client = BuildRequest::new(&self.build_arguments, workspace.clone()).await?;
let default_server = client
.enabled_platforms
.iter()
.any(|p| *p == Platform::Server);

// Make sure we set the fullstack platform so we actually build the fullstack variant
// Users need to enable "fullstack" in their default feature set.
// todo(jon): fullstack *could* be a feature of the app, but right now we're assuming it's always enabled
//
// Now we need to resolve the client features
let fullstack = ((default_server || client.fullstack_feature_enabled())
|| self.fullstack.unwrap_or(false))
&& self.fullstack != Some(false);

if fullstack {
let mut build_args = self.build_arguments.clone();
build_args.platform = Some(Platform::Server);

let _server = BuildRequest::new(&build_args, workspace.clone()).await?;

// ... todo: add the server features to the server build
// ... todo: add the client features to the client build
// // Make sure we have a server feature if we're building a fullstack app
if self.fullstack.unwrap_or_default() && self.server_features.is_empty() {
return Err(anyhow::anyhow!("Fullstack builds require a server feature on the target crate. Add a `server` feature to the crate and try again.").into());
}

server = Some(_server);
}

client
}

// A command in the form of:
// ```
// dx serve \
// client --package frontend \
// server --package backend
// ```
Some(cmd) => {
let mut client_args_ = None;
let mut server_args_ = None;
let mut cmd_outer = Some(Box::new(cmd));
while let Some(cmd) = cmd_outer.take() {
match *cmd {
TargetCmd::Client(cmd_) => {
client_args_ = Some(cmd_.inner);
cmd_outer = cmd_.next;
}
TargetCmd::Server(cmd) => {
server_args_ = Some(cmd.inner);
cmd_outer = cmd.next;
}
}
}

if let Some(server_args) = server_args_ {
server = Some(BuildRequest::new(&server_args, workspace.clone()).await?);
}

BuildRequest::new(&client_args_.unwrap(), workspace.clone()).await?
}
let server_args = match &self.server {
Some(server) => Some(&server.build_arguments),
None => self.shared.default_server(&client),
};

let mut server = None;
// If there is a server, make sure we output in the same directory as the client build so we use the server
// to serve the web client
if let Some(server_args) = server_args {
// Copy the main target from the client to the server
let main_target = client.main_target.clone();
let mut server_args = server_args.clone();
// The platform in the server build is always set to Server
server_args.platform = Some(Platform::Server);
server =
Some(BuildRequest::new(&server_args, Some(main_target), workspace.clone()).await?);
}

Ok(BuildTargets { client, server })
}
}
2 changes: 1 addition & 1 deletion packages/cli/src/cli/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub struct Bundle {

/// The arguments for the dioxus build
#[clap(flatten)]
pub(crate) args: BuildArgs,
pub(crate) args: CommandWithPlatformOverrides<BuildArgs>,
}

impl Bundle {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/cli/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub(crate) struct Check {

/// Information about the target to check
#[clap(flatten)]
pub(crate) build_args: BuildArgs,
pub(crate) build_args: CommandWithPlatformOverrides<BuildArgs>,
}

impl Check {
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ pub(crate) mod config;
pub(crate) mod create;
pub(crate) mod init;
pub(crate) mod link;
pub(crate) mod platform_override;
pub(crate) mod run;
pub(crate) mod serve;
pub(crate) mod target;
Expand All @@ -20,6 +21,7 @@ pub(crate) use serve::*;
pub(crate) use target::*;
pub(crate) use verbosity::*;

use crate::platform_override::CommandWithPlatformOverrides;
use crate::{error::Result, Error, StructuredOutput};
use clap::builder::styling::{AnsiColor, Effects, Style, Styles};
use clap::{Parser, Subcommand};
Expand Down Expand Up @@ -62,7 +64,7 @@ pub(crate) enum Commands {

/// Build the Dioxus project and all of its assets.
#[clap(name = "build")]
Build(build::BuildArgs),
Build(CommandWithPlatformOverrides<build::BuildArgs>),

/// Run the project without any hotreloading.
#[clap(name = "run")]
Expand Down
Loading
Loading