Skip to content
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

Megathread: cli 0.6.0-alpha quirks/issues/ux/perf #2788

Open
2 of 15 tasks
jkelleyrtp opened this issue Aug 5, 2024 · 5 comments
Open
2 of 15 tasks

Megathread: cli 0.6.0-alpha quirks/issues/ux/perf #2788

jkelleyrtp opened this issue Aug 5, 2024 · 5 comments
Labels
bug Something isn't working cli Related to the dioxus-cli program
Milestone

Comments

@jkelleyrtp
Copy link
Member

jkelleyrtp commented Aug 5, 2024

Problem

We have a number of quirks / bugs in the CLI post-rewrite that need to be resolved before releasing to the general public.

This issue is where we put them.

  • Fullstack server takes a moment to start leading to the default-router's "connection refused" screen
  • The tui sidebar makes it impossible to copy logs - needs to be moved to a row CLI Fixes & Tweaks #2846
  • The tui scrolling is just janky/slow/hacky Improved in CLI Fixes & Tweaks #2846
  • The tui "modal" is not complete at all and needs more helpful info (summary of config, urls for dev server, process names, etc)
  • Missing logs for downloading/compiling dependency crates
  • tracing annotations make it harder for vscode to implement jump-to-file
  • Fullstack is just broken due to a port forwarding issue
  • Asset collection causes unnecessary recompiles due to thrashing of env vars in build scripts
  • CWD for bundle (used by icon search) is just wrong leading to Desktop: Error setting the App Icon #2344
  • We don't invalidate literals in hotreloading that are the same as the original, meaning you can hotreload true -> false but not back to true
  • We don't send down old hotreloaded templates to new clients anymore
  • We don't properly set cors/CSP/XSRF headers on assets to clients in dev
  • Collecting and optimizing assets should be optional/skipped in dev mode in favor of just copying assets Revision: Only Optimize Assets In Release #2844
  • we should print the resolved config on a panic in the cli for better debugging
  • wasm-bindgen crashing nukes our TUI
@jkelleyrtp jkelleyrtp added the cli Related to the dioxus-cli program label Aug 5, 2024
@jkelleyrtp jkelleyrtp added this to the 0.6.0 milestone Aug 5, 2024
@jkelleyrtp jkelleyrtp added the bug Something isn't working label Aug 5, 2024
@Andrew15-5
Copy link
Contributor

Andrew15-5 commented Aug 6, 2024

Fullstack is just broken due to a port forwarding issue

Is this the same as (I no longer think it's the same issue and I don't have the port forwarding issue):

  • "Failed to find index.html" Fullstack panic

    [app] thread 'main' panicked at /cargo/git/checkouts/dioxus-1e619ce595d3799d/bd58a92/packages/fullstack/src/serve_config.rs:91:37:
    [app] 2024-08-04T12:04:46.395679Z  WARN dioxus_cli_config: A library is trying to access the crate's configuration, but the dioxus CLI was not used to build the application.
    [app] Failed to find index.html. Make sure the index_path is set correctly and the WASM application has been built.: Os { code: 2, kind: NotFound, message: "No such file or directory" }
    [app] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
    

    ?

    Update 1:

    The commit that introduced this is 0acfe37. Which is weird as it only adds an error/display implementation?

    Update 2:

    After a bit of discussion and messing with crate versions, I was finally able to fix the error:
    I used the latest git version (4c0fefb) and had to patch manganis version (which is pinned to a week-old commit):

    patch
    diff --git a/Cargo.lock b/Cargo.lock
    index 8b5415db4..95e3a3157 100644
    --- a/Cargo.lock
    +++ b/Cargo.lock
    @@ -6076,11 +6076,7 @@ dependencies = [
     [[package]]
     name = "manganis"
     version = "0.3.0-alpha.0"
    -source = "registry+https://github.com/rust-lang/crates.io-index"
    -checksum = "578cb1a3a1a7dfad59feaa34eb928855e7f7d09a031db913b84ac2cc10fdfb84"
    -dependencies = [
    - "manganis-macro",
    -]
    +source = "git+https://github.com/dioxuslabs/manganis#73d51965277729f3098989df1ccfb5fe982ff2a9"
     
     [[package]]
     name = "manganis-cli-support"
    @@ -6127,20 +6123,6 @@ dependencies = [
      "url",
     ]
     
    -[[package]]
    -name = "manganis-macro"
    -version = "0.3.0-alpha.0"
    -source = "registry+https://github.com/rust-lang/crates.io-index"
    -checksum = "046c953865755a7b4e41f3680a2659833b41fdb171a0bffbaeba8b8237319924"
    -dependencies = [
    - "manganis-common",
    - "proc-macro2",
    - "quote",
    - "serde_json",
    - "syn 2.0.72",
    - "tracing-subscriber",
    -]
    -
     [[package]]
     name = "markup5ever"
     version = "0.11.0"
    diff --git a/Cargo.toml b/Cargo.toml
    index b5ed7c723..7888a2b34 100644
    --- a/Cargo.toml
    +++ b/Cargo.toml
    @@ -367,3 +367,7 @@ doc-scrape-examples = true
     name = "window_zoom"
     required-features = ["desktop"]
     doc-scrape-examples = true
    +
    +[patch.crates-io]
    +manganis = { git = "https://github.com/dioxuslabs/manganis" }
    +# manganis-cli-support = { git = "https://github.com/dioxuslabs/manganis" }

    It worked without patching the manganis-cli-support crate. Then I also had to use the PR version of the template + a bit of patching (including re-patching the manganis crate in the fullstack template).

    Update 3:

    After removing manganis dependency in the PR the build was successful! For some reason manganis dependency in dioxus crate doesn't have to be updated.

  • Some messages sometimes are written on top of the UI

    image

    Update:

    Was fixed in 63e7aab.

  • I can't see scrollbar thumb in TUI (maybe it's only visible when the logs are filling out the screen?) in kitty + zellij

    Update:

    Scrollbar thumb only shows up when logs can't fully be shown in the TUI.

  • Add support for a job in the background via Ctrl+Z (have to re-launch dx serve every time or open a new terminal in the same directory)

    Update 1:

    Some problems and progress info here: [RfC] Support for backgrounding an application via Ctrl+Z + fg crossterm-rs/crossterm#494 (comment).

    Update 2:

    feat(cli): added Ctrl+Z (SIGTSTP) support #2836

  • New logs ([app] ... INFO dioxus_fullstack::render: Rebuilding vdom) are appended above the older logs ([dev]), therefore the older ones are being pushed down with each new entry. CLI Fixes & Tweaks #2846

  • Show the time it took for the build/rebuild process to complete (and it's sometimes very useful or to just have an approximate value in your head just in case)
    Either include the time of all steps (build + optimize assets/Wasm etc.) and/or include time for each step (requires more space, but it can look very cool/nice and also be even more useful in benchmarking and stuff; maybe can be enabled with --verbose or something)

  • When the status is build failed the build error is in the [2] build tab, but the CLI changes the tab to [1] console. Same with the [app] logs that are added in the build tab (not visible unless you change the tab). CLI Fixes & Tweaks #2846

  • console and build tab names in the TUI are confusing. Plus, I don't yet understand which logs will go where and do they really need to be separated (some probably do). And additionally, the tab is automatically being switched upon build completion (or failure) while at the same time still putting some logs in the now hidden tab.

  • [r] reload should be renamed to [r] rebuild and the TUI's status: finished should be updated, otherwise I have no idea if anything is working or is it frozen/broken. The only visually understandable cue is the toast in the browser tab (which you can easily miss). Fix: CLI Progress #2840

  • After inspecting the dx init/dx new code I found that it has been changed multiple times since I last touched it. This in turn broke the semantic of the subcommands and also introduced some bugs.

    An example of semantic breakage is the fact that now "name" can accept nested dir paths (a/b/c). An example of bugs is the fact that if "name" and "destination" aren't set then CLI will panic.

    I've started working on rewriting the logic and argument names etc. It won't break simple and mostly used use cases, but will introduce breaking changes in general.

    Update:

    Fix (and rewrite) dx init/dx new #2822

  • Can't use non-default branch when using cargo-generate template URL (to dioxus-template). Specifically, I can't create reproducible example with current version of the CLI while using unmerged PR branch in dioxus-template. Fix (and rewrite) dx init/dx new #2822

  • dx clean doesn't show any progress or how many GiB it has removed. Added dx clean stats #2934

  • I would like this path (and others if they will appear) to be relative to the project root dir. This will make it smaller and more readable. Same for issue logs. So instead it will be [target/dx-dist-name].

    image

@Andrew15-5
Copy link
Contributor

Andrew15-5 commented Aug 7, 2024

I will put non-CLI issues here (but they are emerged from dx serve):

  • After a successful build, removing any of the a {} elements (including the img itself) causes the img/SVG to disappear without the ability to restore it (only via tab reload). OK, any hot-reloadable change makes the image disappear.

        rsx! {
            head::Link { rel: "stylesheet", href: asset!("./assets/main.css") }
            img { src: asset!("./assets/header.svg"), id: "header" }
            div { id: "links",
                a { href: "https://dioxuslabs.com/learn/0.5/", "📚 Learn Dioxus" }
                a { href: "https://dioxuslabs.com/awesome", "🚀 Awesome Dioxus" }
                a { href: "https://github.com/dioxus-community/", "📡 Community Libraries" }
                a { href: "https://github.com/DioxusLabs/dioxus-std", "⚙️ Dioxus Standard Library" }
                a { href: "https://marketplace.visualstudio.com/items?itemName=DioxusLabs.dioxus",
                    "💫 VSCode Extension"
                }
                a { href: "https://discord.gg/XgGxMSkvUM", "👋 Community Discord" }
            }
        }
  • Can't autocomplete the asset path using a generic file-relative (i.e., inside src dir when editing the main.rs file) path autocompletion plugin. This probably would require either changing settings to crate root-relative paths or a dedicated Dioxus LSP server that can provide the correct paths (even using fuzzy find for easier/faster finding of a file) in a certain context (inside asset!("")) and much more (I think a Tailwind CSS support was previously mentioned or something else).

  • After a sequence of asset macro changes (or a single bad one) the final result can be: unapplied CSS that is fixed only with a rebuild.

    I changed ./ in asset!("./assets/header.svg") to ././ and then to ./././ and the page became a white mess after the last rebuild was finished. I don't know if this is just because of the macro or manganis or something else. What is also interesting is that the changed asset string was for an SVG, but I think both of the assets failed to load in the end.

Notes:

  • There was a "quirk" once when I tried to build the project once more. I was surprised since it should've succeed like it did the previous time but instead (After closing the TUI) it showed:

    [dev] 2024-08-07T19:17:43.924546Z  INFO dx::builder::cargo: Copying public assets to the output directory...
    [dev] 2024-08-07T19:17:43.926021Z  INFO dx::builder::web: Running wasm-bindgen
    [dev] 2024-08-07T19:17:43.937072Z0ERROR9dx::builder::web: Bindgen build failed: JoinError::Panic(Id(25), ...)
    [dev] 2024-08-07T19:17:43.937529Z  INFO dx::builder::web: Attempting to recover from bindgen failure by setting the wasm-bindgen version to 0.2.92...thread 'tokio-runtime-worker' panicked at packages/cli/src/build
    [dev] 2024-08-07T19:17:46.275032Z  INFO dx::builder::web: Successfully updated wasm-bindgen to 0.2.92
    

    I thought that it's a one-off problem without any reproduction ability so I didn't copy the additional logs in the TUI (which I thought would all be outside of it, but only a part was outside). Running the same thing for the 3rd time again succeeded with no errors. But in the end, it's worth noting that something like this can happen.

@DogeDark
Copy link
Member

DogeDark commented Aug 14, 2024

Add to the list:

  • Desktop logs aren't outputted in the terminal until the app exits.
  • If the vscode integrated terminal fills up enough to show the scrollbar, the tui will not scroll until the vscode scrollbar reaches its limit. Clearing the terminal does not remove the scrollbar - maybe not fixable. - This is partially fixed except for Windows vscode terminal. CLI Fixes & Tweaks #2846

@FragrantArmpit
Copy link
Contributor

Another thing to discuss: #2857

@dbcfd
Copy link

dbcfd commented Sep 3, 2024

Fullstack is just broken due to a port forwarding issue

Is this the same as (I no longer think it's the same issue and I don't have the port forwarding issue):

* [x]  "Failed to find index.html" Fullstack panic
  ```
  [app] thread 'main' panicked at /cargo/git/checkouts/dioxus-1e619ce595d3799d/bd58a92/packages/fullstack/src/serve_config.rs:91:37:
  [app] 2024-08-04T12:04:46.395679Z  WARN dioxus_cli_config: A library is trying to access the crate's configuration, but the dioxus CLI was not used to build the application.
  [app] Failed to find index.html. Make sure the index_path is set correctly and the WASM application has been built.: Os { code: 2, kind: NotFound, message: "No such file or directory" }
  [app] note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  ```

I have been able to work around this in 0.6.0-alpha.2 by setting out_dir to dist/public. It looks like DioxusLabs/dioxus-template#44 should be adjusted to use this since

let index_path = self
will use public_path() if index_path is not set.

Update: This only works once. If dist/public exists, then next build will try to find the index in dist/public/public. It seems you need a build with out_dir for dist/public then set out_dir back to dist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Related to the dioxus-cli program
Projects
None yet
Development

No branches or pull requests

5 participants