فهرست منبع

Fix WSL Hot Reload (#2721)

* feat: poll watcher
* progress: wsl hot reload setting
* feat: wsl poll setting
Miles Murgaw 11 ماه پیش
والد
کامیت
b7127ad6ae
5فایلهای تغییر یافته به همراه159 افزوده شده و 62 حذف شده
  1. 33 30
      packages/cli/src/cli/config.rs
  2. 16 1
      packages/cli/src/cli/serve.rs
  3. 1 1
      packages/cli/src/serve/mod.rs
  4. 89 29
      packages/cli/src/serve/watcher.rs
  5. 20 1
      packages/cli/src/settings.rs

+ 33 - 30
packages/cli/src/cli/config.rs

@@ -26,51 +26,47 @@ pub enum Config {
     /// Create a custom html file.
     CustomHtml {},
 
-    /// Set global cli settings.
-    SetGlobal { setting: Setting, value: Value },
+    /// Set CLI settings.
+    #[command(subcommand)]
+    Set(Setting),
 }
 
-#[derive(Debug, Clone, Copy, Deserialize, clap::ValueEnum)]
+#[derive(Debug, Clone, Copy, Deserialize, Subcommand)]
 pub enum Setting {
     /// Set the value of the always-hot-reload setting.
-    AlwaysHotReload,
+    AlwaysHotReload { value: BoolValue },
     /// Set the value of the always-open-browser setting.
-    AlwaysOpenBrowser,
+    AlwaysOpenBrowser { value: BoolValue },
     /// Set the value of the always-on-top desktop setting.
-    AlwaysOnTop,
+    AlwaysOnTop { value: BoolValue },
+    /// Set the interval that file changes are polled on WSL for hot reloading.
+    WSLFilePollInterval { value: u16 },
 }
 
 impl Display for Setting {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::AlwaysHotReload => write!(f, "always_hot_reload"),
-            Self::AlwaysOpenBrowser => write!(f, "always_open_browser"),
-            Self::AlwaysOnTop => write!(f, "always_on_top"),
+            Self::AlwaysHotReload { value: _ } => write!(f, "always-hot-reload"),
+            Self::AlwaysOpenBrowser { value: _ } => write!(f, "always-open-browser"),
+            Self::AlwaysOnTop { value: _ } => write!(f, "always-on-top"),
+            Self::WSLFilePollInterval { value: _ } => write!(f, "wsl-file-poll-interval"),
         }
     }
 }
 
-// NOTE: Unsure of an alternative to get the desired behavior with clap, if it exists.
+// Clap complains if we use a bool directly and I can't find much info about it.
+// "Argument 'value` is positional and it must take a value but action is SetTrue"
 #[derive(Debug, Clone, Copy, Deserialize, clap::ValueEnum)]
-pub enum Value {
+pub enum BoolValue {
     True,
     False,
 }
 
-impl Display for Value {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Self::True => write!(f, "true"),
-            Self::False => write!(f, "false"),
-        }
-    }
-}
-
-impl From<Value> for bool {
-    fn from(value: Value) -> Self {
+impl From<BoolValue> for bool {
+    fn from(value: BoolValue) -> Self {
         match value {
-            Value::True => true,
-            Value::False => false,
+            BoolValue::True => true,
+            BoolValue::False => false,
         }
     }
 }
@@ -111,14 +107,21 @@ impl Config {
                 file.write_all(content.as_bytes())?;
                 tracing::info!("🚩 Create custom html file done.");
             }
-            // Handle configuration of global CLI settings.
-            Config::SetGlobal { setting, value } => {
+            // Handle CLI settings.
+            Config::Set(setting) => {
                 CliSettings::modify_settings(|settings| match setting {
-                    Setting::AlwaysHotReload => settings.always_hot_reload = Some(value.into()),
-                    Setting::AlwaysOpenBrowser => settings.always_open_browser = Some(value.into()),
-                    Setting::AlwaysOnTop => settings.always_on_top = Some(value.into()),
+                    Setting::AlwaysOnTop { value } => settings.always_on_top = Some(value.into()),
+                    Setting::AlwaysHotReload { value } => {
+                        settings.always_hot_reload = Some(value.into())
+                    }
+                    Setting::AlwaysOpenBrowser { value } => {
+                        settings.always_open_browser = Some(value.into())
+                    }
+                    Setting::WSLFilePollInterval { value } => {
+                        settings.wsl_file_poll_interval = Some(value)
+                    }
                 })?;
-                tracing::info!("🚩 CLI setting `{setting}` has been set to `{value}`")
+                tracing::info!("🚩 CLI setting `{setting}` has been set.");
             }
         }
         Ok(())

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

@@ -36,6 +36,10 @@ pub struct ServeArguments {
     /// Additional arguments to pass to the executable
     #[clap(long)]
     pub args: Vec<String>,
+
+    /// Sets the interval in seconds that the CLI will poll for file changes on WSL.
+    #[clap(long, default_missing_value = "2")]
+    pub wsl_file_poll_interval: Option<u16>,
 }
 
 /// Run the WASM project on dev-server
@@ -59,15 +63,26 @@ pub struct Serve {
 impl Serve {
     /// Resolve the serve arguments from the arguments or the config
     fn resolve(&mut self, crate_config: &mut DioxusCrate) -> Result<()> {
-        // Set config settings
+        // Set config settings.
         let settings = settings::CliSettings::load();
 
+        // Enable hot reload.
         if self.server_arguments.hot_reload.is_none() {
             self.server_arguments.hot_reload = Some(settings.always_hot_reload.unwrap_or(true));
         }
+
+        // Open browser.
         if self.server_arguments.open.is_none() {
             self.server_arguments.open = Some(settings.always_open_browser.unwrap_or_default());
         }
+
+        // Set WSL file poll interval.
+        if self.server_arguments.wsl_file_poll_interval.is_none() {
+            self.server_arguments.wsl_file_poll_interval =
+                Some(settings.wsl_file_poll_interval.unwrap_or(2));
+        }
+
+        // Set always-on-top for desktop.
         if self.server_arguments.always_on_top.is_none() {
             self.server_arguments.always_on_top = Some(settings.always_on_top.unwrap_or(true))
         }

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

@@ -52,7 +52,7 @@ pub async fn serve_all(serve: Serve, dioxus_crate: DioxusCrate) -> Result<()> {
     builder.build();
 
     let mut server = Server::start(&serve, &dioxus_crate);
-    let mut watcher = Watcher::start(&dioxus_crate);
+    let mut watcher = Watcher::start(&serve, &dioxus_crate);
     let mut screen = Output::start(&serve).expect("Failed to open terminal logger");
 
     loop {

+ 89 - 29
packages/cli/src/serve/watcher.rs

@@ -1,13 +1,16 @@
-use std::path::PathBuf;
+use std::{fs, path::PathBuf, time::Duration};
 
-use crate::dioxus_crate::DioxusCrate;
 use crate::serve::hot_reloading_file_map::FileMap;
+use crate::{cli::serve::Serve, dioxus_crate::DioxusCrate};
 use dioxus_hot_reload::HotReloadMsg;
 use dioxus_html::HtmlCtx;
 use futures_channel::mpsc::{UnboundedReceiver, UnboundedSender};
 use futures_util::StreamExt;
 use ignore::gitignore::Gitignore;
-use notify::{event::ModifyKind, EventKind, RecommendedWatcher};
+use notify::{
+    event::{MetadataKind, ModifyKind},
+    Config, EventKind,
+};
 
 /// This struct stores the file watcher and the filemap for the project.
 ///
@@ -17,14 +20,14 @@ pub struct Watcher {
     _tx: UnboundedSender<notify::Event>,
     rx: UnboundedReceiver<notify::Event>,
     _last_update_time: i64,
-    _watcher: RecommendedWatcher,
+    _watcher: Box<dyn notify::Watcher>,
     queued_events: Vec<notify::Event>,
     file_map: FileMap,
     ignore: Gitignore,
 }
 
 impl Watcher {
-    pub fn start(config: &DioxusCrate) -> Self {
+    pub fn start(serve: &Serve, config: &DioxusCrate) -> Self {
         let (tx, rx) = futures_channel::mpsc::unbounded();
 
         // Extend the watch path to include:
@@ -61,27 +64,41 @@ impl Watcher {
         }
         let ignore = builder.build().unwrap();
 
-        // Create the file watcher
-        let mut watcher = notify::recommended_watcher({
+        // Build the event handler for notify.
+        let notify_event_handler = {
             let tx = tx.clone();
             move |info: notify::Result<notify::Event>| {
                 if let Ok(e) = info {
-                     match e.kind {
-
-                        // An event emitted when the metadata of a file or folder is changed.
-                        EventKind::Modify(ModifyKind::Data(_) | ModifyKind::Any) |
-                        EventKind::Create(_) |
-                        EventKind::Remove(_) => {
-                            _ = tx.unbounded_send(e);
-                        },
-                        _ => {}
+                    if is_allowed_notify_event(&e) {
+                        _ = tx.unbounded_send(e);
                     }
-
-
                 }
             }
-        })
-        .expect("Failed to create file watcher.\nEnsure you have the required permissions to watch the specified directories.");
+        };
+
+        // If we are in WSL, we must use Notify's poll watcher due to an event propagation issue.
+        let is_wsl = is_wsl();
+        const NOTIFY_ERROR_MSG: &str = "Failed to create file watcher.\nEnsure you have the required permissions to watch the specified directories.";
+
+        // Create the file watcher.
+        let mut watcher: Box<dyn notify::Watcher> = match is_wsl {
+            true => {
+                let poll_interval = Duration::from_secs(
+                    serve.server_arguments.wsl_file_poll_interval.unwrap_or(2) as u64,
+                );
+
+                Box::new(
+                    notify::PollWatcher::new(
+                        notify_event_handler,
+                        Config::default().with_poll_interval(poll_interval),
+                    )
+                    .expect(NOTIFY_ERROR_MSG),
+                )
+            }
+            false => {
+                Box::new(notify::recommended_watcher(notify_event_handler).expect(NOTIFY_ERROR_MSG))
+            }
+        };
 
         // Watch the specified paths
         // todo: make sure we don't double-watch paths if they're nested
@@ -95,7 +112,6 @@ impl Watcher {
 
             let mode = notify::RecursiveMode::Recursive;
 
-            use notify::Watcher;
             if let Err(err) = watcher.watch(path, mode) {
                 tracing::warn!("Failed to watch path: {}", err);
             }
@@ -144,14 +160,9 @@ impl Watcher {
 
         // Decompose the events into a list of all the files that have changed
         for event in self.queued_events.drain(..) {
-            // We only care about modify/crate/delete events
-            match event.kind {
-                EventKind::Modify(ModifyKind::Any) => {}
-                EventKind::Modify(ModifyKind::Data(_)) => {}
-                EventKind::Modify(ModifyKind::Name(_)) => {}
-                EventKind::Create(_) => {}
-                EventKind::Remove(_) => {}
-                _ => continue,
+            // We only care about certain events.
+            if !is_allowed_notify_event(&event) {
+                continue;
             }
 
             for path in event.paths {
@@ -276,6 +287,55 @@ fn is_backup_file(path: PathBuf) -> bool {
     false
 }
 
+/// Tests if the provided [`notify::Event`] is something we listen to so we can avoid unescessary hot reloads.
+fn is_allowed_notify_event(event: &notify::Event) -> bool {
+    match event.kind {
+        EventKind::Modify(ModifyKind::Data(_)) => true,
+        EventKind::Modify(ModifyKind::Name(_)) => true,
+        EventKind::Create(_) => true,
+        EventKind::Remove(_) => true,
+        // The primary modification event on WSL's poll watcher.
+        EventKind::Modify(ModifyKind::Metadata(MetadataKind::WriteTime)) => true,
+        // Catch-all for unknown event types.
+        EventKind::Modify(ModifyKind::Any) => true,
+        // Don't care about anything else.
+        _ => false,
+    }
+}
+
+const WSL_1: &str = "/proc/sys/kernel/osrelease";
+const WSL_2: &str = "/proc/version";
+const WSL_KEYWORDS: [&str; 2] = ["microsoft", "wsl"];
+
+/// Detects if `dx` is being ran in a WSL environment.
+///
+/// We determine this based on whether the keyword `microsoft` or `wsl` is contained within the [`WSL_1`] or [`WSL_2`] files.
+/// This may fail in the future as it isn't guaranteed by Microsoft.
+/// See https://github.com/microsoft/WSL/issues/423#issuecomment-221627364
+fn is_wsl() -> bool {
+    // Test 1st File
+    if let Ok(content) = fs::read_to_string(WSL_1) {
+        let lowercase = content.to_lowercase();
+        for keyword in WSL_KEYWORDS {
+            if lowercase.contains(keyword) {
+                return true;
+            }
+        }
+    }
+
+    // Test 2nd File
+    if let Ok(content) = fs::read_to_string(WSL_2) {
+        let lowercase = content.to_lowercase();
+        for keyword in WSL_KEYWORDS {
+            if lowercase.contains(keyword) {
+                return true;
+            }
+        }
+    }
+
+    false
+}
+
 #[test]
 fn test_is_backup_file() {
     assert!(is_backup_file(PathBuf::from("examples/test.rs~")));

+ 20 - 1
packages/cli/src/settings.rs

@@ -25,6 +25,9 @@ pub struct CliSettings {
     pub always_open_browser: Option<bool>,
     /// Describes whether desktop apps in development will be pinned always-on-top.
     pub always_on_top: Option<bool>,
+    /// Describes the interval in seconds that the CLI should poll for file changes on WSL.
+    #[serde(default = "default_wsl_file_poll_interval")]
+    pub wsl_file_poll_interval: Option<u16>,
 }
 
 impl CliSettings {
@@ -74,7 +77,19 @@ impl CliSettings {
             CrateConfigError::Io(Error::new(ErrorKind::Other, e.to_string()))
         })?;
 
-        let result = fs::write(path.clone(), data.clone());
+        // Create the directory structure if it doesn't exist.
+        let parent_path = path.parent().unwrap();
+        if let Err(e) = fs::create_dir_all(parent_path) {
+            error!(
+                ?data,
+                ?path,
+                "failed to create directories for settings file"
+            );
+            return Err(CrateConfigError::Io(e));
+        }
+
+        // Write the data.
+        let result = fs::write(&path, data.clone());
         if let Err(e) = result {
             error!(?data, ?path, "failed to save global cli settings");
             return Err(CrateConfigError::Io(e));
@@ -102,3 +117,7 @@ impl CliSettings {
         Ok(())
     }
 }
+
+fn default_wsl_file_poll_interval() -> Option<u16> {
+    Some(2)
+}