Maintenance patch.

[martinfrances107]

Ran the standard battery of checks and fixed up deprectaed issues, clippy warnings etc.

cargo update
cargo outdated
cargo fmt
cargo clippy

Lots of packges updated. In particular lightningcss moved from 47-55(alpha) This is a essential first step in resolving outstanding lightningcss issues.

Hightlights :-

• Clippy now making use of .flatten()

• Fixed 5 deprecated issues of all of the form :-

  ```console warning: use of deprecated macro assert_display_snapshot: use assert_snapshot!() instead --> src/compile/tests.rs:62:5 |
  62 | assert_display_snapshot!(cargo, @"cargo build --package=example --bin=example --no-default-features --features=ssr");
  | ^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(deprecated)]` on by default

PS Notify is 2 major number behind .. I am going to create a separate PR with Notifty only changes.

[martinfrances107]

Just making notes while I figure this out....

Here are two errors related to the window build
I dont have a window machine, so I can't recreate the issue locally -
So I am going to have to think my way through the problem

error[E0432]: unresolved import `winapi::shared::winerror`
  --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-4.0.17\src\windows.rs:11:21
   |
11 | use winapi::shared::winerror::ERROR_OPERATION_ABORTED;
   |                     ^^^^^^^^ could not find `winerror` in `shared`

   Compiling reqwest v0.12.4
error[E0308]: mismatched types
   --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\notify-4.0.17\src\windows.rs:276:29
    |
276 |         overlapped.hEvent = request_p;
    |         -----------------   ^^^^^^^^^ expected `winapi::ctypes::c_void`, found `libc::c_void`
    |         |
    |         expected due to the type of this binding
    |
    = note: `libc::c_void` and `winapi::ctypes::c_void` have similar names, but are actually distinct types
note: `libc::c_void` is defined in crate `core`
   --> /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04\library\core\src\ffi\mod.rs:173:1
note: `winapi::ctypes::c_void` is defined in crate `winapi`
   --> C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\winapi-0.3.9\src\lib.rs:38:5
    |
38  |     pub enum c_void {}
    |     ^^^^^^^^^^^^^^^

Reverse engineering the break

Change to "brotli" triggers changes to "flexilogger"
Changes to "flexilogger" triggers changes to "chrono"
Change to "chrono" triggers changes to "windows_targets"

So I think reverting to the change to brotli would fix the issue
But I want somthing cleaner.

[martinfrances107]

The last commit is experimental .. more work is required.

[shivjm]

I’m on Windows 10 and I can confirm the winapi build issue. Installing martinfrances107/cargo-leptos@98ca5bed works, but it looks like watching files isn’t functional, or at least isn’t functioning correctly, because I see a lot of these events without any sort of rebuilding (unless I restart the command, of course):

       Event { kind: Modify(Any), paths: ["C:\\Media\\src\\my\\hledger-web-rs\\src\\app.rs"], attr:tracker: None, attr:flag: None, attr:info: None, attr:source: None }

[martinfrances107]

Updated version of notify is a major change.

In this new regime "Rename" events can supply a variable number of files.

Now notify is cross platform and has lots of new fine grained events.

I am not sure I have preserved the intent of the original.. I am still looking at this.

[gbj]

Thanks! I am not extremely familiar with the notification code here, but I think the new version preserves the same behavior.

It does look (at least from CI) like the Windows build issue is solved, right?

Do you feel like this one is ready? Having read through it, nothing jumps out at me.

[shivjm]

Just chiming in once more to say that cargo leptos watch still isn’t responding to file changes for me with martinfrances107/cargo-leptos@558bef52. My apologies if this is out of scope; I only started using Leptos with this branch, so I don’t know whether it’s changed behaviour or not.

[martinfrances107]

I will have another look, this issue started as a maintence patch.

I tried to do the changes associated with the notfiy crate in an isolated PR as it was a project in itself.

I am a big fan of keeping PRs small and isolated to one issue... BUT... here the slowly increasing complexity is justified.

regarding the changes in notify.rs try_new() -- it lacks associated tests.

So NEXT .. I am going to add the smallest test to trigger on change of file contents.

[martinfrances107]

The next PR is a outline of the test

// Overivew :-
//
// SETUP: create a file in a valid project.
//
// 1) Construct watching mechanism.
//
// 2) Modfify the file.
//
// 3) Assert the mechanism observed a valid event.
//
// TEARDOWN: delete the file.
#[ignore]
#[test]
fn change_file_contents() {
  /// ....
}

I am currently "#[ignore]ing" the new test - its not passing, I am in the middle of debugging it.

But I see enough detail to cause an architectural discussion in the background.

from the commit log -

"A structural issue, if no event is generated
then the test hangs."

I am going the have to insert a timeout mechansim that says
after N ms -- The event is declared overdue and a fail issued.

Making test dependant on a minimim execution time is borderline
bad practise ... We are making assumption about the resouces
given to the testing infrasture..

But in this case I think an exception has to be made?

[martinfrances107]

Cogitating on this some more . and unpicking this some more. I am about the generate the next commit
Test are still not passing.

From the original main branch

    let (sync_tx, sync_rx) = std::sync::mpsc::channel::<DebouncedEvent>();

So next, I am restoring the debounce behaviour.

In this new updated crate environment the choice is "debounce-mini" or "notify-debouncer-full".

"debounce-mini" events do not contain the required information about the filename.

debounced events are prefered as they are less chatty -- less events for us to process.

In formal language .. just for reference here is what the notify-debounce-full crate has to say on this..

//! A debouncer for [notify] that is optimized for ease of use.
//!
//! * Only emits a single `Rename` event if the rename `From` and `To` events can be matched
//! * Merges multiple `Rename` events
//! * Takes `Rename` events into account and updates paths for events that occurred before the rename event, but which haven't been emitted, yet
//! * Optionally keeps track of the file system IDs all files and stiches rename events together (FSevents, Windows)
//! * Emits only one `Remove` event when deleting a directory (inotify)
//! * Doesn't emit duplicate create events
//! * Doesn't emit `Modify` events after a `Create` event

---

BugFix...

I could start using a crate that implements a TEARDOWN feature but I am trying to keep things simple.
I am using the TEARDOWN feature once ...and so to fix an issue I am now awkwardly tearing down before the actual assert.

---

NB a variable name change - "watcher" becomes "debouncer"

• if let Err(e) = watcher.watch(path, RecursiveMode::Recursive) {

• if let Err(e) = debouncer.watcher().watch(path.as_std_path(), RecursiveMode::Recursive) {

otherwise watcher.watcher().watch() is just a stutter.

[martinfrances107]

The new test passes on Linux, but not windows.

The new test can be viewed a framework for drilling down and identifying the Events that windows is issuing and I am not seeing on my OS.

My current thought is there are lots of fine grained Events like ModifyKind::Other
which windows is issuing and we are not handling correctly

[martinfrances107]

Nuts, I thought I had covered all the corner cases...

At the moment I can't see a way forward...

More on the weekend.

[martinfrances107]

The expected debug is missing from build artefacts - implying no events are emitted.

This warning/filename conflict is interesting -- could the file creation / modification be silently failing !!!

warning: output filename collision.
The bin target `project2` in package `project2 v0.1.0 (D:\a\cargo-leptos\cargo-leptos\examples\workspace\project2)` has the same output filename as the lib target `project2` in package `project2 v0.1.0 (D:\a\cargo-leptos\cargo-leptos\examples\workspace\project2)`.
Colliding filename is: D:\a\cargo-leptos\cargo-leptos\examples\workspace\target\debug\deps\project2.pdb
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
```

[martinfrances107]

I am about to publish a new commit.

The only clue in the test artefacts is a "project conflict" [ project2 ] which I was reusing for the new test.

So my response is to hive off a new cargo project, named "notify" and use that soley for the test [ change_file_contents()].

In terms of review .. I think that this is going to have to be refactored .. but I want something that passes.

[martinfrances107]

So after that isolation refactor .. those conflict warnings persist, and so they are not related to the new test..

The debug message I added shows "events" are being generated...

So it looks to me, the root problem is a "window" specific misconfiguration

Hmm, I am not sure what to try next.

[gbj]

I haven't gone back through and re-read everything after my comments a couple weeks ago.

@shivjm Could you clarify: If you cargo install --locked, that should install the currently published version successfully. If you do that, does cargo leptos watch work?

If it does work on the current, published version then I guess it's something in the update/PR that breaks file-watching on Windows in particular.

If it does not work on the current, published version, then it seems unrelated to this PR and maybe more specific to your setup (although since the test is failing in Windows CI, that is maybe less likely)

Thanks!

[shivjm]

@gbj The published version works perfectly with --locked, which I hadn’t tried before. In that version, cargo leptos watch is able to detect changes and reload automatically as it should. Thanks!

Without --locked, I get the winapi error from earlier in this thread and can’t install the package. With this branch, no changes are detected.

[martinfrances107]

Checked cargo outdated again ... fixed a couple of issues.

[martinfrances107]

From my comments above

I am going the have to insert a timeout mechansim that says
after N ms -- The event is declared overdue and a fail issued.

Making test dependant on a minimim execution time is borderline
bad practise ... We are making assumption about the resouces
given to the testing infrasture..

I have now set the watchdog timeout to 4s .. this well above what I think is reasonable.

[martinfrances107]

Hmm, that maybe a intresting data point

Now the new test fails in "macros-latest" ... that is when I updat the dependencies within the examples directory. I wonder if I can use this is identify the sub-module that is causing the failure on windows but not on linux....Hope fully it will be a small list.

[benwis]

Do you think this is ready for merge?

[martinfrances107]

Do you think this is ready for merge?

No, the only remaining issues is why does the new test "change_file_contents()" pass on ubuntu but timesout of Windows.

PS thanks for all the micro merges recently .. The PR here is smaller. I need more time to reason this out..

[martinfrances107]

#295 has just demonstrated that bumping for zip isolation is problematic for Windows targets only.

The next step here is to back out changes to the zip crate so they can be dealt with in isolation
.. on sec

[martinfrances107]

I am still working on this as and when I get a new idea.
Sorry if it coming out in slowly and with so much experimentation

Here is the current state of things.

based on shivjm reporting .. on his setup windows issues an event which we consume correclty ... It the windows tests only, which are broken.For some reason no event is generated.

So that has me looking the notify crates doc for windows specific config that I may be missing

this new test is the first tokio test ( see #[tokio::test] )

Maybe instead of windows specific it is tokio specific?

one sec will submit a new commit based on
https://docs.rs/notify/6.1.1/notify/#crossbeam-channel--tokio