From 8b5ad5d925555c38fe2e6d823c51587e54424cb9 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Tue, 23 Jul 2024 09:58:13 -0700 Subject: [PATCH] Fix some CI (windows + nasm, core-macro error) (#2676) * Moves index.html creation until after wasm creation so the webserver doesn't prematurely serve the page * sets up nasm for windows builds (to be superseded by Remove Dioxus CLI NASM dependency #2666 #2682) * puts magnanis-cli-support in a blocking task to prevent threadlocking during sequential builds * changes order of client/server builds such that the client is built before the server so the webserver doesn't prematurely serve invalid client code * declobbers "serve" such that each project gets a different dir in the target folder * uses nest_service instead of fallback so the router doesn't respond with fallback codes * reincorporates doge's fix from Fix hot-reloading on Windows #2544 --- .github/workflows/main.yml | 4 ++++ .gitignore | 2 ++ packages/cli/src/builder/cargo.rs | 16 +++++++++++----- packages/cli/src/builder/fullstack.rs | 2 +- packages/cli/src/builder/web.rs | 13 ++++++++----- packages/cli/src/cli/serve.rs | 10 ++++++++++ packages/cli/src/serve/mod.rs | 9 ++++----- packages/cli/src/serve/output.rs | 2 +- packages/cli/src/serve/server.rs | 4 ++-- .../core-macro/tests/rsx/trailing-comma-0.stderr | 3 +-- packages/fullstack/src/serve_config.rs | 1 + packages/rsx/src/literal.rs | 14 +++++--------- packages/rsx/src/template_body.rs | 15 +++++---------- 13 files changed, 55 insertions(+), 40 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3e1cb356c..054311c3f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -269,6 +269,10 @@ jobs: targets: ${{ matrix.platform.target }} components: rustfmt + - name: Install nasm for windows (tls) + if: ${{ matrix.platform.target == 'x86_64-pc-windows-msvc' }} + uses: ilammy/setup-nasm@v1 + - name: Install cross if: ${{ matrix.platform.cross == true }} diff --git a/.gitignore b/.gitignore index f7156b125..4974ac19b 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ /dist .DS_Store /examples/assets/test_video.mp4 +static # new recommendation to keep the lockfile in for CI and reproducible builds # Cargo.lock @@ -22,3 +23,4 @@ node_modules/ /test-results/ /packages/playwright-report/ /packages/playwright/.cache/ + diff --git a/packages/cli/src/builder/cargo.rs b/packages/cli/src/builder/cargo.rs index 0439163ba..113a10474 100644 --- a/packages/cli/src/builder/cargo.rs +++ b/packages/cli/src/builder/cargo.rs @@ -150,11 +150,17 @@ impl BuildRequest { // Start Manganis linker intercept. let linker_args = vec![format!("{}", self.dioxus_crate.out_dir().display())]; - manganis_cli_support::start_linker_intercept( - &LinkCommand::command_name(), - cargo_args, - Some(linker_args), - )?; + // Don't block the main thread - magnanis should not be running its own std process but it's + // fine to wrap it here at the top + tokio::task::spawn_blocking(move || { + manganis_cli_support::start_linker_intercept( + &LinkCommand::command_name(), + cargo_args, + Some(linker_args), + ) + }) + .await + .unwrap()?; let file_name = self.dioxus_crate.executable_name(); diff --git a/packages/cli/src/builder/fullstack.rs b/packages/cli/src/builder/fullstack.rs index 141c6e26f..e39939429 100644 --- a/packages/cli/src/builder/fullstack.rs +++ b/packages/cli/src/builder/fullstack.rs @@ -44,8 +44,8 @@ impl BuildRequest { serve: bool, ) -> Vec { vec![ - Self::new_server(serve, &config, &build_arguments), Self::new_client(serve, &config, &build_arguments), + Self::new_server(serve, &config, &build_arguments), ] } diff --git a/packages/cli/src/builder/web.rs b/packages/cli/src/builder/web.rs index 3524d727e..7530f44a5 100644 --- a/packages/cli/src/builder/web.rs +++ b/packages/cli/src/builder/web.rs @@ -120,11 +120,6 @@ impl BuildRequest { update: UpdateStage::Start, }); - // Create the index.html file - let html = self.prepare_html(assets)?; - let html_path = self.dioxus_crate.out_dir().join("index.html"); - std::fs::write(&html_path, html)?; - // Find the wasm file let output_location = build_result.executable.clone(); let input_path = output_location.with_extension("wasm"); @@ -177,6 +172,14 @@ impl BuildRequest { .await .unwrap()?; + // Create the index.html file + // Note that we do this last since the webserver will attempt to serve the index.html file + // If we do this too early, the wasm won't be ready but the index.html will be served, leading + // to test failures and broken pages. + let html = self.prepare_html(assets)?; + let html_path = self.dioxus_crate.out_dir().join("index.html"); + std::fs::write(html_path, html)?; + Ok(()) } } diff --git a/packages/cli/src/cli/serve.rs b/packages/cli/src/cli/serve.rs index 4183af9ce..212292c38 100644 --- a/packages/cli/src/cli/serve.rs +++ b/packages/cli/src/cli/serve.rs @@ -68,6 +68,16 @@ impl Serve { // Resolve the build arguments self.build_arguments.resolve(crate_config)?; + // Since this is a serve, adjust the outdir to be target/dx-dist/ + let mut dist_dir = crate_config.workspace_dir().join("target").join("dx-dist"); + + if crate_config.target.is_example() { + dist_dir = dist_dir.join("examples"); + } + + crate_config.dioxus_config.application.out_dir = + dist_dir.join(crate_config.executable_name()); + Ok(()) } diff --git a/packages/cli/src/serve/mod.rs b/packages/cli/src/serve/mod.rs index a0580c762..38a24d7fb 100644 --- a/packages/cli/src/serve/mod.rs +++ b/packages/cli/src/serve/mod.rs @@ -45,16 +45,15 @@ use watcher::*; /// - I want us to be able to detect a `server_fn` in the project and then upgrade from a static server /// to a dynamic one on the fly. pub async fn serve_all(serve: Serve, dioxus_crate: DioxusCrate) -> Result<()> { - let mut server = Server::start(&serve, &dioxus_crate); - let mut watcher = Watcher::start(&dioxus_crate); - let mut screen = Output::start(&serve) - .await - .expect("Failed to open terminal logger"); let mut builder = Builder::new(&dioxus_crate, &serve); // Start the first build builder.build(); + let mut server = Server::start(&serve, &dioxus_crate); + let mut watcher = Watcher::start(&dioxus_crate); + let mut screen = Output::start(&serve).expect("Failed to open terminal logger"); + loop { // Make sure we don't hog the CPU: these loop { select! {} } blocks can starve the executor yield_now().await; diff --git a/packages/cli/src/serve/output.rs b/packages/cli/src/serve/output.rs index 9569c7bed..3707c8abc 100644 --- a/packages/cli/src/serve/output.rs +++ b/packages/cli/src/serve/output.rs @@ -86,7 +86,7 @@ enum Tab { type TerminalBackend = Terminal>; impl Output { - pub async fn start(cfg: &Serve) -> io::Result { + pub fn start(cfg: &Serve) -> io::Result { let interactive = std::io::stdout().is_tty() && cfg.interactive.unwrap_or(true); let mut events = None; diff --git a/packages/cli/src/serve/server.rs b/packages/cli/src/serve/server.rs index 465218827..f98eb2fd2 100644 --- a/packages/cli/src/serve/server.rs +++ b/packages/cli/src/serve/server.rs @@ -328,13 +328,13 @@ fn setup_router( match platform { Platform::Web => { // Route file service to output the .wasm and assets if this is a web build - router = router.fallback(build_serve_dir(serve, config)); + router = router.nest_service("/", build_serve_dir(serve, config)); } Platform::Fullstack | Platform::StaticGeneration => { // For fullstack and static generation, forward all requests to the server let address = fullstack_address.unwrap(); - router = router.fallback(super::proxy::proxy_to( + router = router.nest_service("/",super::proxy::proxy_to( format!("http://{address}").parse().unwrap(), true, |error| { diff --git a/packages/core-macro/tests/rsx/trailing-comma-0.stderr b/packages/core-macro/tests/rsx/trailing-comma-0.stderr index f7909cd04..5b316b477 100644 --- a/packages/core-macro/tests/rsx/trailing-comma-0.stderr +++ b/packages/core-macro/tests/rsx/trailing-comma-0.stderr @@ -1,7 +1,6 @@ error: Attributes must be separated by commas + = help: Did you forget a comma? --> tests/rsx/trailing-comma-0.rs:9:13 | 9 | class: "foo bar" | ^^^^^ - | - = help: Did you forget a comma? diff --git a/packages/fullstack/src/serve_config.rs b/packages/fullstack/src/serve_config.rs index b4d828b9a..0d80aa325 100644 --- a/packages/fullstack/src/serve_config.rs +++ b/packages/fullstack/src/serve_config.rs @@ -139,6 +139,7 @@ pub(crate) struct IndexHtml { #[derive(Clone)] pub struct ServeConfig { pub(crate) index: IndexHtml, + #[allow(unused)] pub(crate) assets_path: PathBuf, pub(crate) incremental: Option, } diff --git a/packages/rsx/src/literal.rs b/packages/rsx/src/literal.rs index e92252f9e..5175119ee 100644 --- a/packages/rsx/src/literal.rs +++ b/packages/rsx/src/literal.rs @@ -160,15 +160,11 @@ impl ToTokens for HotLiteral { // But the key is what's keeping it stable GlobalSignal::with_key( || #val, { - concat!( - file!(), - ":", - line!(), - ":", - column!(), - ":", - #hr_idx - ) + { + const PATH: &str = dioxus_core::const_format::str_replace!(file!(), "\\\\", "/"); + const NORMAL: &str = dioxus_core::const_format::str_replace!(PATH, '\\', "/"); + dioxus_core::const_format::concatcp!(NORMAL, ':', line!(), ':', column!(), ':', #hr_idx) + } }) .maybe_with_rt(|s| s #mapped) } diff --git a/packages/rsx/src/template_body.rs b/packages/rsx/src/template_body.rs index 76e4de008..92bd73cd0 100644 --- a/packages/rsx/src/template_body.rs +++ b/packages/rsx/src/template_body.rs @@ -181,22 +181,17 @@ impl ToTokens for TemplateBody { .iter() .map(|(path, idx)| self.get_dyn_attr(path, *idx).rendered_as_dynamic_attr()); - // Rust analyzer will not autocomplete properly if we change the name every time you type a character - // If it looks like we are running in rust analyzer, we'll just use a placeholder location - // let looks_like_rust_analyzer = first_root_span.contains("SpanData"); - // let index = if looks_like_rust_analyzer { - // "0".to_string() - // } else { - // self.template_idx.get().to_string() - // }; - // todo: this just might be fixed? let index = self.template_idx.get(); tokens.append_all(quote! { dioxus_core::Element::Ok({ #[doc(hidden)] // vscode please stop showing these in symbol search static ___TEMPLATE: dioxus_core::Template = dioxus_core::Template { - name: concat!( file!(), ":", line!(), ":", column!(), ":", #index ) , + name: { + const PATH: &str = dioxus_core::const_format::str_replace!(file!(), "\\\\", "/"); + const NORMAL: &str = dioxus_core::const_format::str_replace!(PATH, '\\', "/"); + dioxus_core::const_format::concatcp!(NORMAL, ':', line!(), ':', column!(), ':', #index) + }, roots: &[ #( #roots ),* ], node_paths: &[ #( #node_paths ),* ], attr_paths: &[ #( #attr_paths ),* ],