Browse Source

Remove default features when fullstack is enabled and one platform is enabled in the default features (#3925)

* Don't enable default features when only either the server or client is enabled
Evan Almloff 2 tháng trước cách đây
mục cha
commit
8c866b783d

+ 7 - 0
Cargo.lock

@@ -4104,6 +4104,13 @@ dependencies = [
  "once_cell",
 ]
 
+[[package]]
+name = "dioxus-playwright-default-features-disabled-test"
+version = "0.1.0"
+dependencies = [
+ "dioxus",
+]
+
 [[package]]
 name = "dioxus-playwright-fullstack-mounted-test"
 version = "0.1.0"

+ 1 - 0
Cargo.toml

@@ -103,6 +103,7 @@ members = [
     "packages/playwright-tests/nested-suspense",
     "packages/playwright-tests/cli-optimization",
     "packages/playwright-tests/wasm-split-harness",
+    "packages/playwright-tests/default-features-disabled"
 ]
 
 [workspace.package]

+ 1 - 1
packages/cli/src/build/request.rs

@@ -92,7 +92,7 @@ impl BuildRequest {
     pub(crate) async fn build_server(&self) -> Result<Option<BuildArtifacts>> {
         tracing::debug!("Building server...");
 
-        if !self.build.fullstack {
+        if !self.build.fullstack() {
             return Ok(None);
         }
 

+ 25 - 12
packages/cli/src/cli/build.rs

@@ -31,8 +31,8 @@ pub(crate) struct BuildArgs {
     /// Build the fullstack variant of this app, using that as the fileserver and backend
     ///
     /// This defaults to `false` but will be overridden to true if the `fullstack` feature is enabled.
-    #[clap(long)]
-    pub(crate) fullstack: bool,
+    #[arg(long, default_missing_value="true", num_args=0..=1)]
+    pub(crate) fullstack: Option<bool>,
 
     /// Run the ssg config of the app and generate the files
     #[clap(long)]
@@ -91,12 +91,25 @@ impl BuildArgs {
     /// IE if they've specified "fullstack" as a feature on `dioxus`, then we want to build the
     /// fullstack variant even if they omitted the `--fullstack` flag.
     pub(crate) async fn resolve(&mut self, krate: &DioxusCrate) -> Result<()> {
-        let default_platform = krate.default_platform();
+        let default_platforms = krate.default_platforms();
+        let default_platform = default_platforms.iter().find(|p| **p != Platform::Server);
+        let default_server = default_platforms.iter().any(|p| *p == Platform::Server);
         let auto_platform = krate.autodetect_platform();
 
-        // The user passed --platform XYZ but already has `default = ["ABC"]` in their Cargo.toml
-        // We want to strip out the default platform and use the one they passed, setting no-default-features
-        if self.platform.is_some() && default_platform.is_some() {
+        // Make sure we set the fullstack platform so we actually build the fullstack variant
+        // Users need to enable "fullstack" in their default feature set or explicitly pass the flag
+        self.fullstack = Some(
+            self.fullstack()
+                || self.fullstack.is_none()
+                    && (default_server || krate.has_dioxus_feature("fullstack")),
+        );
+
+        // If the current build is a fullstack build which includes either the client or the server in the default features,
+        // remove that default feature and just add it back into the client or server args. If they passed in an explicit platform
+        // but they also have a default feature platform, strip out the default features and add back in the platform they passed in.
+        if self.fullstack() && (default_server || default_platform.is_some())
+            || self.platform.is_some() && default_platform.is_some()
+        {
             self.target_args.no_default_features = true;
             self.target_args
                 .features
@@ -126,16 +139,11 @@ impl BuildArgs {
             .server_features
             .push(krate.feature_for_platform(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
-        self.fullstack = self.fullstack || krate.has_dioxus_feature("fullstack");
-
         // Make sure we have a server feature if we're building a fullstack app
         //
         // todo(jon): eventually we want to let users pass a `--server <crate>` flag to specify a package to use as the server
         // however, it'll take some time to support that and we don't have a great RPC binding layer between the two yet
-        if self.fullstack && self.target_args.server_features.is_empty() {
+        if self.fullstack() && self.target_args.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());
         }
 
@@ -191,4 +199,9 @@ impl BuildArgs {
     pub(crate) fn platform(&self) -> Platform {
         self.platform.expect("Platform was not set")
     }
+
+    /// Check if this is a fullstack build
+    pub(crate) fn fullstack(&self) -> bool {
+        self.fullstack.unwrap_or(false)
+    }
 }

+ 1 - 1
packages/cli/src/cli/bundle.rs

@@ -54,7 +54,7 @@ impl Bundle {
         let mut bundles = vec![];
 
         // Copy the server over if it exists
-        if bundle.build.build.fullstack {
+        if bundle.build.build.fullstack() {
             bundles.push(bundle.server_exe().unwrap());
         }
 

+ 1 - 1
packages/cli/src/cli/run.rs

@@ -27,7 +27,7 @@ impl RunArgs {
         let fullstack_ip = "127.0.0.1:8080".parse().unwrap();
         let mut open_address = None;
 
-        if self.build_args.platform() == Platform::Web || self.build_args.fullstack {
+        if self.build_args.platform() == Platform::Web || self.build_args.fullstack() {
             open_address = Some(fullstack_ip);
             tracing::info!("Serving at: {}", fullstack_ip);
         }

+ 1 - 1
packages/cli/src/cli/serve.rs

@@ -104,7 +104,7 @@ impl ServeArgs {
         match self.build_arguments.platform() {
             Platform::Server => true,
             // During SSG, just serve the static files instead of running the server
-            _ => self.build_arguments.fullstack && !self.build_arguments.ssg,
+            _ => self.build_arguments.fullstack() && !self.build_arguments.ssg,
         }
     }
 }

+ 15 - 8
packages/cli/src/dioxus_crate.rs

@@ -252,7 +252,9 @@ impl DioxusCrate {
             .iter()
             .flat_map(|feature| {
                 tracing::trace!("Autodetecting platform from feature {feature}");
-                Platform::autodetect_from_cargo_feature(feature).map(|f| (f, feature.to_string()))
+                Platform::autodetect_from_cargo_feature(feature)
+                    .filter(|platform| *platform != Platform::Server)
+                    .map(|f| (f, feature.to_string()))
             })
             .collect::<Vec<_>>();
 
@@ -459,8 +461,11 @@ impl DioxusCrate {
             .map(|krate| krate.krate.version.to_string())
     }
 
-    pub(crate) fn default_platform(&self) -> Option<Platform> {
-        let default = self.package().features.get("default")?;
+    pub(crate) fn default_platforms(&self) -> Vec<Platform> {
+        let Some(default) = self.package().features.get("default") else {
+            return Vec::new();
+        };
+        let mut platforms = vec![];
 
         // we only trace features 1 level deep..
         for feature in default.iter() {
@@ -468,8 +473,8 @@ impl DioxusCrate {
             if feature.starts_with("dioxus/") {
                 let dx_feature = feature.trim_start_matches("dioxus/");
                 let auto = Platform::autodetect_from_cargo_feature(dx_feature);
-                if auto.is_some() {
-                    return auto;
+                if let Some(auto) = auto {
+                    platforms.push(auto);
                 }
             }
 
@@ -480,15 +485,17 @@ impl DioxusCrate {
                     if feature.starts_with("dioxus/") {
                         let dx_feature = feature.trim_start_matches("dioxus/");
                         let auto = Platform::autodetect_from_cargo_feature(dx_feature);
-                        if auto.is_some() {
-                            return auto;
+                        if let Some(auto) = auto {
+                            platforms.push(auto);
                         }
                     }
                 }
             }
         }
 
-        None
+        platforms.sort();
+        platforms.dedup();
+        platforms
     }
 
     /// Gather the features that are enabled for the package

+ 2 - 2
packages/cli/src/serve/output.rs

@@ -497,7 +497,7 @@ impl Output {
             state.build_engine.compile_duration(),
         );
 
-        if state.build_engine.request.build.fullstack {
+        if state.build_engine.request.build.fullstack() {
             self.render_single_gauge(
                 frame,
                 second_progress,
@@ -651,7 +651,7 @@ impl Output {
             Paragraph::new(Line::from(vec![
                 "Platform: ".gray(),
                 self.platform.expected_name().yellow(),
-                if state.opts.build_arguments.fullstack {
+                if state.opts.build_arguments.fullstack() {
                     " + fullstack".yellow()
                 } else {
                     " ".dark_gray()

+ 11 - 0
packages/playwright-tests/default-features-disabled.spec.js

@@ -0,0 +1,11 @@
+// @ts-check
+const { test, expect } = require("@playwright/test");
+
+test("loads with correct features", async ({ page }) => {
+  await page.goto("http://localhost:8002");
+
+  // Expect the page to contain the pending text.
+  const main = page.locator("#main");
+  await expect(main).toContainText('server features: ["server"]');
+  await expect(main).toContainText('client features: ["web"]');
+});

+ 3 - 0
packages/playwright-tests/default-features-disabled/.gitignore

@@ -0,0 +1,3 @@
+.dioxus
+dist
+target

+ 16 - 0
packages/playwright-tests/default-features-disabled/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "dioxus-playwright-default-features-disabled-test"
+version = "0.1.0"
+edition = "2021"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+dioxus = { workspace = true, features = ["fullstack"] }
+
+[features]
+# Web is a default feature, but it shouldn't actually be enabled in server builds
+default = ["web"]
+server = ["dioxus/server"]
+web = ["dioxus/web"]

+ 42 - 0
packages/playwright-tests/default-features-disabled/src/main.rs

@@ -0,0 +1,42 @@
+// This test asserts that the client feature is disable on the server build by the cli
+// even if it is set as a default feature
+
+#![allow(non_snake_case)]
+use dioxus::prelude::*;
+
+fn main() {
+    launch(app);
+}
+
+fn app() -> Element {
+    let server_features = use_server_future(get_server_features)?.unwrap().unwrap();
+    let mut client_features = use_signal(Vec::new);
+    use_effect(move || {
+        client_features.set(current_platform_features());
+    });
+
+    rsx! {
+        div {
+            "server features: {server_features:?}"
+        }
+        div {
+            "client features: {client_features:?}"
+        }
+    }
+}
+
+fn current_platform_features() -> Vec<String> {
+    let mut features = Vec::new();
+    if cfg!(feature = "web") {
+        features.push("web".to_string());
+    }
+    if cfg!(feature = "server") {
+        features.push("server".to_string());
+    }
+    features
+}
+
+#[server]
+async fn get_server_features() -> Result<Vec<String>, ServerFnError> {
+    Ok(current_platform_features())
+}

+ 9 - 0
packages/playwright-tests/playwright.config.js

@@ -166,5 +166,14 @@ module.exports = defineConfig({
       reuseExistingServer: !process.env.CI,
       stdout: "pipe",
     },
+    {
+      cwd: path.join(process.cwd(), "default-features-disabled"),
+      command:
+        'cargo run --package dioxus-cli --release -- serve --force-sequential --addr "127.0.0.1" --port 8002',
+      port: 8002,
+      timeout: 50 * 60 * 1000,
+      reuseExistingServer: !process.env.CI,
+      stdout: "pipe",
+    },
   ],
 });