From d559fdaeab7a3f69541aae0c61e6d6672f348e9e Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Sat, 12 Aug 2023 19:19:46 -0500 Subject: [PATCH 1/5] fix borrowed props with temp values --- .../props_safety_temporary_values.rs | 24 +++++++++++++++++++ .../props_safety_temporary_values.stderr | 12 ++++++++++ packages/core/src/properties.rs | 1 + packages/core/src/scopes.rs | 4 +++- 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 packages/core/compile_tests/props_safety_temporary_values.rs create mode 100644 packages/core/compile_tests/props_safety_temporary_values.stderr diff --git a/packages/core/compile_tests/props_safety_temporary_values.rs b/packages/core/compile_tests/props_safety_temporary_values.rs new file mode 100644 index 000000000..c332f63c4 --- /dev/null +++ b/packages/core/compile_tests/props_safety_temporary_values.rs @@ -0,0 +1,24 @@ +use dioxus::prelude::*; + +fn main() {} + +fn app(cx: Scope) -> Element { + let count = vec![1, 2, 3]; + + render! { + unsafe_child_component { + borrowed: &count + } + } +} + +#[derive(Props)] +struct Testing<'a> { + borrowed: &'a Vec, +} + +fn unsafe_child_component<'a>(cx: Scope<'a, Testing<'a>>) -> Element<'a> { + cx.render(rsx! { + div { "{cx.props.borrowed:?}" } + }) +} diff --git a/packages/core/compile_tests/props_safety_temporary_values.stderr b/packages/core/compile_tests/props_safety_temporary_values.stderr new file mode 100644 index 000000000..c1ef443c9 --- /dev/null +++ b/packages/core/compile_tests/props_safety_temporary_values.stderr @@ -0,0 +1,12 @@ +error[E0515]: cannot return value referencing local variable `count` + --> compile_tests/props_safety_temporary_values.rs:8:5 + | +8 | / render! { +9 | | unsafe_child_component { +10 | | borrowed: &count + | | ------ `count` is borrowed here +11 | | } +12 | | } + | |_____^ returns a value referencing data owned by the current function + | + = note: this error originates in the macro `render` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/packages/core/src/properties.rs b/packages/core/src/properties.rs index f754f8b68..3f0d9ef97 100644 --- a/packages/core/src/properties.rs +++ b/packages/core/src/properties.rs @@ -79,4 +79,5 @@ pub fn fc_to_builder<'a, T: Properties + 'a>(_: fn(Scope<'a, T>) -> Element<'a>) fn unsafe_props_fail() { let t = trybuild::TestCases::new(); t.compile_fail("compile_tests/props_safety.rs"); + t.compile_fail("compile_tests/props_safety_temporary_values.rs"); } diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 5ada89069..17e499d62 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -424,7 +424,9 @@ impl<'src> ScopeState { fn_name: &'static str, ) -> DynamicNode<'src> where - P: Properties + 'child, + // The properties must be valid until the next bump frame + P: Properties + 'src, + // The current bump allocator frame must outlive the child's borrowed props 'src: 'child, { let vcomp = VProps::new(component, P::memoize, props); From 882c3c9dfb6abd061263bbaeaf162633a315762a Mon Sep 17 00:00:00 2001 From: Yolo <92775570+HashCookie@users.noreply.github.com> Date: Tue, 22 Aug 2023 23:39:22 +0800 Subject: [PATCH 2/5] Update ZH_CN.md to use 0.4 version of guide (#1388) * fix: readme awesome link * Update ZH_CN.md to use 0.4 version of guide --- notes/README/ZH_CN.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notes/README/ZH_CN.md b/notes/README/ZH_CN.md index 1c4abc667..45bf82f09 100644 --- a/notes/README/ZH_CN.md +++ b/notes/README/ZH_CN.md @@ -40,7 +40,7 @@ | 代码示例 | - 开发指南 + 开发指南 | English | From da4794d7937ae9d554a1be7e2d7b5dfe6645e80d Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Tue, 22 Aug 2023 11:56:13 -0700 Subject: [PATCH 3/5] bump core to 0.4.2 --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5b55f4628..252e9c83e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,12 +50,12 @@ members = [ exclude = ["examples/mobile_demo"] [workspace.package] -version = "0.4.1" +version = "0.4.2" # dependencies that are shared across packages [workspace.dependencies] dioxus = { path = "packages/dioxus", version = "0.4.0" } -dioxus-core = { path = "packages/core", version = "0.4.1" } +dioxus-core = { path = "packages/core", version = "0.4.2" } dioxus-core-macro = { path = "packages/core-macro", version = "0.4.0" } dioxus-router = { path = "packages/router", version = "0.4.1" } dioxus-router-macro = { path = "packages/router-macro", version = "0.4.1" } From de87ba6548adbf5af08a3148da50e5ea07668083 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Tue, 22 Aug 2023 20:48:03 -0400 Subject: [PATCH 4/5] Update all uncommented commands to remove actions-rs (#1366) * Update all uncommented commands to remove actions-rs (#1) * install target in toolchain action * add linux target explicitly * try latest * fix install --- .github/workflows/main.yml | 46 ++++++++-------- .github/workflows/playwright.yml | 71 ++++++++++++------------- packages/cli/.github/workflows/main.yml | 34 ++++-------- 3 files changed, 65 insertions(+), 86 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c6e14ab6f..04f367b8e 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -85,52 +85,56 @@ jobs: matrix_test: runs-on: ${{ matrix.platform.os }} + env: + RUST_CARGO_COMMAND: ${{ matrix.platform.cross == true && 'cross' || 'cargo' }} strategy: matrix: platform: - { target: x86_64-pc-windows-msvc, os: windows-latest, - toolchain: '1.70.0', + toolchain: "1.70.0", cross: false, - command: 'test', - args: '--all --tests' + command: "test", + args: "--all --tests", } - { target: x86_64-apple-darwin, os: macos-latest, - toolchain: '1.70.0', + toolchain: "1.70.0", cross: false, - command: 'test', - args: '--all --tests' + command: "test", + args: "--all --tests", } - { target: aarch64-apple-ios, os: macos-latest, - toolchain: '1.70.0', + toolchain: "1.70.0", cross: false, - command: 'build', - args: '--package dioxus-mobile' + command: "build", + args: "--package dioxus-mobile", } - { target: aarch64-linux-android, os: ubuntu-latest, - toolchain: '1.70.0', + toolchain: "1.70.0", cross: true, - command: 'build', - args: '--package dioxus-mobile' + command: "build", + args: "--package dioxus-mobile", } steps: - uses: actions/checkout@v3 - name: install stable - uses: actions-rs/toolchain@v1 + uses: dtolnay/rust-toolchain@master with: toolchain: ${{ matrix.platform.toolchain }} - target: ${{ matrix.platform.target }} - override: true - default: true + targets: ${{ matrix.platform.target }} + + - name: Install cross + if: ${{ matrix.platform.cross == true }} + uses: taiki-e/install-action@cross - uses: Swatinem/rust-cache@v2 with: @@ -138,13 +142,8 @@ jobs: save-if: ${{ matrix.features.key == 'all' }} - name: test - uses: actions-rs/cargo@v1 - with: - use-cross: ${{ matrix.platform.cross }} - command: ${{ matrix.platform.command }} - args: --target ${{ matrix.platform.target }} ${{ matrix.platform.args }} - - + run: | + ${{ env.RUST_CARGO_COMMAND }} ${{ matrix.platform.command }} ${{ matrix.platform.args }} --target ${{ matrix.platform.target }} # Coverage is disabled until we can fix it # coverage: @@ -166,4 +165,3 @@ jobs: # uses: codecov/codecov-action@v2 # with: # fail_ci_if_error: false - diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 41fa92fa7..dba32d257 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -1,9 +1,9 @@ name: Playwright Tests on: push: - branches: [ main, master ] + branches: [main, master] pull_request: - branches: [ main, master ] + branches: [main, master] defaults: run: working-directory: ./playwright-tests @@ -16,39 +16,36 @@ jobs: test: if: github.event.pull_request.draft == false timeout-minutes: 60 - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - # Do our best to cache the toolchain and node install steps - - uses: actions/checkout@v3 - - uses: actions/setup-node@v3 - with: - node-version: 16 - - name: Install Rust - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true - - uses: Swatinem/rust-cache@v2 - - name: Install WASM toolchain - run: rustup target add wasm32-unknown-unknown - - name: Install dependencies - run: npm ci - - name: Install Playwright - run: npm install -D @playwright/test - - name: Install Playwright Browsers - run: npx playwright install --with-deps - # Cache the CLI by using cargo run internally - # - name: Install Dioxus CLI - # uses: actions-rs/cargo@v1 - # with: - # command: install - # args: --path packages/cli - - name: Run Playwright tests - run: npx playwright test - - uses: actions/upload-artifact@v3 - if: always() - with: - name: playwright-report - path: playwright-report/ - retention-days: 30 + # Do our best to cache the toolchain and node install steps + - uses: actions/checkout@v3 + - uses: actions/setup-node@v3 + with: + node-version: 16 + - name: Install Rust + uses: dtolnay/rust-toolchain@master + with: + toolchain: stable + targets: x86_64-unknown-linux-gnu,wasm32-unknown-unknown + - uses: Swatinem/rust-cache@v2 + - name: Install dependencies + run: npm ci + - name: Install Playwright + run: npm install -D @playwright/test + - name: Install Playwright Browsers + run: npx playwright install --with-deps + # Cache the CLI by using cargo run internally + # - name: Install Dioxus CLI + # uses: actions-rs/cargo@v1 + # with: + # command: install + # args: --path packages/cli + - name: Run Playwright tests + run: npx playwright test + - uses: actions/upload-artifact@v3 + if: always() + with: + name: playwright-report + path: playwright-report/ + retention-days: 30 diff --git a/packages/cli/.github/workflows/main.yml b/packages/cli/.github/workflows/main.yml index f978ba7a2..b22f78eae 100644 --- a/packages/cli/.github/workflows/main.yml +++ b/packages/cli/.github/workflows/main.yml @@ -8,47 +8,31 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true + - name: Install Rust + uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v1 - - uses: actions-rs/cargo@v1 - with: - command: check + - run: cargo check test: name: Test Suite runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true + - name: Install Rust + uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v1 - - uses: actions-rs/cargo@v1 - with: - command: test + - run: cargo test fmt: name: Rustfmt runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true + - name: Install Rust + uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v1 - run: rustup component add rustfmt - - uses: actions-rs/cargo@v1 - with: - command: fmt - args: --all -- --check + - run: cargo fmt --all -- --check # clippy: # name: Clippy From 3ac2346e46afa5fa790c50c2415c3f85e7fa6229 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Fri, 25 Aug 2023 07:54:04 -0500 Subject: [PATCH 5/5] fix a few new clippy lints --- packages/router/src/contexts/router.rs | 34 ++++++++++++-------------- packages/web/src/dom.rs | 1 + 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/router/src/contexts/router.rs b/packages/router/src/contexts/router.rs index 5a9a3f0c4..b15956991 100644 --- a/packages/router/src/contexts/router.rs +++ b/packages/router/src/contexts/router.rs @@ -2,7 +2,7 @@ use std::{ any::Any, collections::HashSet, rc::Rc, - sync::{Arc, RwLock, RwLockWriteGuard}, + sync::{Arc, RwLock}, }; use dioxus::prelude::*; @@ -36,7 +36,7 @@ struct MutableRouterState { /// A collection of router data that manages all routing functionality. #[derive(Clone)] pub struct RouterContext { - state: Arc>, + state: Rc>, subscribers: Arc>>, subscriber_update: Arc, @@ -56,7 +56,7 @@ impl RouterContext { R: Clone, ::Err: std::fmt::Display, { - let state = Arc::new(RwLock::new(MutableRouterState { + let state = Rc::new(RefCell::new(MutableRouterState { prefix: Default::default(), history: cfg.take_history(), unresolved_error: None, @@ -105,7 +105,7 @@ impl RouterContext { // set the updater { - let mut state = myself.state.write().unwrap(); + let mut state = myself.state.borrow_mut(); state.history.updater(Arc::new(move || { for &id in subscribers.read().unwrap().iter() { (mark_dirty)(id); @@ -117,20 +117,20 @@ impl RouterContext { } pub(crate) fn route_from_str(&self, route: &str) -> Result, String> { - let state = self.state.read().unwrap(); + let state = self.state.borrow(); state.history.parse_route(route) } /// Check whether there is a previous page to navigate back to. #[must_use] pub fn can_go_back(&self) -> bool { - self.state.read().unwrap().history.can_go_back() + self.state.borrow().history.can_go_back() } /// Check whether there is a future page to navigate forward to. #[must_use] pub fn can_go_forward(&self) -> bool { - self.state.read().unwrap().history.can_go_forward() + self.state.borrow().history.can_go_forward() } /// Go back to the previous location. @@ -138,7 +138,7 @@ impl RouterContext { /// Will fail silently if there is no previous location to go to. pub fn go_back(&self) { { - self.state.write().unwrap().history.go_back(); + self.state.borrow_mut().history.go_back(); } self.change_route(); @@ -149,7 +149,7 @@ impl RouterContext { /// Will fail silently if there is no next location to go to. pub fn go_forward(&self) { { - self.state.write().unwrap().history.go_forward(); + self.state.borrow_mut().history.go_forward(); } self.change_route(); @@ -206,8 +206,7 @@ impl RouterContext { /// The route that is currently active. pub fn current(&self) -> R { self.state - .read() - .unwrap() + .borrow() .history .current_route() .downcast::() @@ -218,7 +217,7 @@ impl RouterContext { /// The route that is currently active. pub fn current_route_string(&self) -> String { - self.any_route_to_string(&*self.state.read().unwrap().history.current_route()) + self.any_route_to_string(&*self.state.borrow().history.current_route()) } pub(crate) fn any_route_to_string(&self, route: &dyn Any) -> String { @@ -243,7 +242,7 @@ impl RouterContext { /// The prefix that is currently active. pub fn prefix(&self) -> Option { - self.state.read().unwrap().prefix.clone() + self.state.borrow().prefix.clone() } fn external(&self, external: String) -> Option { @@ -261,8 +260,8 @@ impl RouterContext { } } - fn state_mut(&self) -> RwLockWriteGuard { - self.state.write().unwrap() + fn state_mut(&self) -> RefMut { + self.state.borrow_mut() } /// Manually subscribe to the current route @@ -283,15 +282,14 @@ impl RouterContext { /// Clear any unresolved errors pub fn clear_error(&self) { - self.state.write().unwrap().unresolved_error = None; + self.state.borrow_mut().unresolved_error = None; self.update_subscribers(); } pub(crate) fn render_error<'a>(&self, cx: Scope<'a>) -> Element<'a> { self.state - .read() - .unwrap() + .borrow() .unresolved_error .as_ref() .and_then(|_| (self.failure_external_navigation)(cx)) diff --git a/packages/web/src/dom.rs b/packages/web/src/dom.rs index 9d0d837c5..52f2ab1a9 100644 --- a/packages/web/src/dom.rs +++ b/packages/web/src/dom.rs @@ -392,6 +392,7 @@ fn read_input_to_data(target: Element) -> Rc { .dyn_ref() .and_then(|input: &web_sys::HtmlInputElement| { input.files().and_then(|files| { + #[allow(clippy::arc_with_non_send_sync)] crate::file_engine::WebFileEngine::new(files) .map(|f| std::sync::Arc::new(f) as std::sync::Arc) })