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

Add a bunch more anti patterns #269

Merged
merged 1 commit into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ dioxus-std = { git = "https://github.com/ealmloff/dioxus-std", branch = "copy-Us
], optional = true }
tower-http = { version = "0.5.0", optional = true, features = ["timeout"] }
tracing = "0.1.40"
rand = { version = "0.8.5", optional = true }

[patch.crates-io]
dioxus = { git = "https://github.com/dioxuslabs/dioxus" }
Expand Down Expand Up @@ -131,6 +132,7 @@ doc_test = [
"tower-http",
"dioxus-std",
"http",
"rand"
]
web = ["dioxus-web", "dioxus/web", "dioxus/web", "dioxus-fullstack/web"]
server = [
Expand Down
45 changes: 44 additions & 1 deletion docs-src/0.5/en/cookbook/antipatterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,51 @@ While it is technically acceptable to have a `Mutex` or a `RwLock` in the props,

Suppose you have a struct `User` containing the field `username: String`. If you pass a `Mutex<User>` prop to a `UserComponent` component, that component may wish to write to the `username` field. However, when it does, the parent component will not be aware of the change, and the component will not re-render which causes the UI to be out of sync with the state. Instead, consider passing down a reactive value like a `Signal` or immutable data.

```rust
{{#include src/doc_examples/anti_patterns.rs:interior_mutability}}
```

## Avoid Updating State During Render

Every time you update the state, Dioxus needs to re-render the component – this is inefficient! Consider refactoring your code to avoid this.

Also, if you unconditionally update the state during render, it will be re-rendered in an infinite loop.
Also, if you unconditionally update the state during render, it will be re-rendered in an infinite loop.

```rust
{{#include src/doc_examples/anti_patterns.rs:updating_state}}
```

## Avoid Large Groups of State

It can be tempting to have a single large state struct that contains all of your application's state. However, this can lead to issues:
- It can be easy to accidentally mutate the state in a way that causes an infinite loop
- It can be difficult to reason about when and how the state is updated
- It can lead to performance issues because many components will need to re-render when the state changes

Instead, consider breaking your state into smaller, more manageable pieces. This will make it easier to reason about the state, avoid update loops, and improve performance.

```rust
{{#include src/doc_examples/anti_patterns.rs:large_state}}
```

## Running Non-Deterministic Code in the Body of a Component

If you have a component that contains non-deterministic code, that code should generally not be run in the body of the component. If it is put in the body of the component, it will be executed every time the component is re-rendered which can lead to performance issues.

Instead, consider moving the non-deterministic code into a hook that only runs when the component is first created or an effect that reruns when dependencies change.

```rust
{{#include src/doc_examples/anti_patterns.rs:non_deterministic}}
```

## Overly Permissive PartialEq for Props

You may have noticed that `Props` requires a `PartialEq` implementation. That `PartialEq` is very important for Dioxus to work correctly. It is used to determine if a component should re-render or not when the parent component re-renders.

If you cannot derive `PartialEq` for your `Props`, you will need to implement it yourself. If you do implement `PartialEq`, make sure to return `false` any time the props change in a way that would cause the UI in the child component to change.

In general, returning `false` from `PartialEq` if you aren't sure if the props have changed or not is better than returning `true`. This will help you avoid out of date UI in your child components.

```rust
{{#include src/doc_examples/anti_patterns.rs:permissive_partial_eq}}
```
286 changes: 286 additions & 0 deletions src/doc_examples/anti_patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//! This example shows what *not* to do

use std::collections::HashMap;
use std::cell::RefCell;
use std::rc::Rc;

use dioxus::prelude::*;

Expand Down Expand Up @@ -65,3 +67,287 @@ fn AntipatternNoKeys(props: NoKeysProps) -> Element {
}
// ANCHOR_END: iter_keys
}


// ANCHOR: interior_mutability
// ❌ Mutex/RwLock/RefCell in props
#[derive(Props, Clone)]
struct AntipatternInteriorMutability {
map: Rc<RefCell<HashMap<u32, String>>>,
}

impl PartialEq for AntipatternInteriorMutability {
fn eq(&self, other: &Self) -> bool {
std::rc::Rc::ptr_eq(&self.map, &other.map)
}
}

fn AntipatternInteriorMutability(map: Rc<RefCell<HashMap<u32, String>>>) -> Element {
rsx! {
button {
onclick: {
let map = map.clone();
move |_| {
// Writing to map will not rerun any components
map.borrow_mut().insert(0, "Hello".to_string());
}
},
"Mutate map"
}
// Since writing to map will not rerun any components, this will get out of date
"{map.borrow().get(&0).unwrap()}"
}
}

// ✅ Use a signal to pass mutable state
#[component]
fn AntipatternInteriorMutabilitySignal(map: Signal<HashMap<u32, String>>) -> Element {
rsx! {
button {
onclick: move |_| {
// Writing to map will rerun any components that read the map
map.write().insert(0, "Hello".to_string());
},
"Mutate map"
}
// Since writing to map will rerun subscribers, this will get updated
"{map.read().get(&0).unwrap()}"
}
}
// ANCHOR_END: interior_mutability

fn AntipatternUpdatingState() -> Element {
// ANCHOR: updating_state
// ❌ Updating state in render
let first_signal = use_signal(|| 0);
let mut second_signal = use_signal(|| 0);

// Updating the state during a render can easily lead to infinite loops
if first_signal() + 1 != second_signal() {
second_signal.set(first_signal() + 1);
}

// ✅ Update state in an effect
let first_signal = use_signal(|| 0);
let mut second_signal = use_signal(|| 0);

// The closure you pass to use_effect will be rerun whenever any of the dependencies change without re-rendering the component
use_effect(move || {
if first_signal() + 1 != second_signal() {
second_signal.set(first_signal() + 1);
}
});

// ✅ Deriving state with use_memo
let first_signal = use_signal(|| 0);
// Memos are specifically designed for derived state. If your state fits this pattern, use it.
let second_signal = use_memo(move || first_signal() + 1);
// ANCHOR_END: updating_state
todo!()
}

// ANCHOR: large_state
fn app() -> Element {
// ❌ Large state struct
#[derive(Props, Clone, PartialEq)]
struct LargeState {
users: Vec<User>,
logged_in: bool,
warnings: Vec<String>,
}

#[derive(Props, Clone, PartialEq)]
struct User {
name: String,
email: String,
}

let mut all_my_state = use_signal(|| LargeState {
users: vec![User {
name: "Alice".to_string(),
email: "[email protected]".to_string(),
}],
logged_in: true,
warnings: vec![],
});

use_effect(move || {
// It is very easy to accidentally read and write to the state object if it contains all your state
let read = all_my_state.read();
let logged_in = read.logged_in;
if !logged_in {
all_my_state.write_unchecked().warnings.push("You are not logged in".to_string());
}
});

// ✅ Use multiple signals to manage state
let users = use_signal(|| vec![User {
name: "Alice".to_string(),
email: "[email protected]".to_string(),
}]);
let logged_in = use_signal(|| true);
let mut warnings = use_signal(|| vec![]);

use_effect(move || {
// Now you can read and write to separate signals which will not cause issues
if !logged_in() {
warnings.write().push("You are not logged in".to_string());
}
});

// ✅ Use memos to create derived state when larger states are unavoidable
// Notice we didn't split everything into separate signals. Users still make sense as a vec of data
let users = use_signal(|| vec![User {
name: "Alice".to_string(),
email: "[email protected]".to_string(),
}]);
let logged_in = use_signal(|| true);
let warnings: Signal<Vec<String>> = use_signal(|| vec![]);

// In child components, you can use the memo to create derived that will only update when a specific part of the state changes
// This will help you avoid unnecessary re-renders and infinite loops
#[component]
fn FirstUser(users: Signal<Vec<User>>) -> Element {
let first_user = use_memo(move || users.read().first().unwrap().clone());

rsx! {
div {
"First user: {first_user().name}"
}
}
}

rsx! {
FirstUser {
users
}
}
}
// ANCHOR_END: large_state


// ANCHOR: non_deterministic
// ❌ Non-deterministic code in the body of a component
#[component]
fn NonDeterministic(name: String) -> Element {
let my_random_id = rand::random::<u64>();

rsx! {
div {
// Id will change every single time the component is re-rendered
id: "{my_random_id}",
"Hello {name}"
}
}
}

// ✅ Use a hook to run non-deterministic code
fn NonDeterministicHook(name: String) -> Element {
// If you store the result of the non-deterministic code in a hook, it will stay the same between renders
let my_random_id = use_hook(|| rand::random::<u64>());

rsx! {
div {
id: "{my_random_id}",
"Hello {name}"
}
}
}
// ANCHOR_END: non_deterministic

// ANCHOR: permissive_partial_eq
// ❌ Permissive PartialEq for Props
#[derive(Props, Clone)]
struct PermissivePartialEqProps {
name: String,
}

// This will cause the component to **never** re-render when the parent component re-renders
impl PartialEq for PermissivePartialEqProps {
fn eq(&self, _: &Self) -> bool {
true
}
}

fn PermissivePartialEq(name: PermissivePartialEqProps) -> Element {
rsx! {
div {
"Hello {name.name}"
}
}
}

#[component]
fn PermissivePartialEqParent() -> Element {
let name = use_signal(|| "Alice".to_string());

rsx! {
PermissivePartialEq {
// The PermissivePartialEq component will not get the updated value of name because the PartialEq implementation says that the props are the same
name: name()
}
}
}

// ✅ Derive PartialEq for Props
#[derive(Props, Clone, PartialEq)]
struct DerivePartialEqProps {
name: String,
}

fn DerivePartialEq(name: DerivePartialEqProps) -> Element {
rsx! {
div {
"Hello {name.name}"
}
}
}

#[component]
fn DerivePartialEqParent() -> Element {
let name = use_signal(|| "Alice".to_string());

rsx! {
DerivePartialEq {
name: name()
}
}
}

// ✅ Return false from PartialEq if you are unsure if the props have changed
#[derive(Debug)]
struct NonPartialEq;

#[derive(Props, Clone)]
struct RcPartialEqProps {
name: Rc<NonPartialEq>,
}

impl PartialEq for RcPartialEqProps {
fn eq(&self, other: &Self) -> bool {
// This will almost always return false because the Rc will likely point to a different value
// Implementing PartialEq for NonPartialEq would be better, but if it is controlled by another library, it may not be possible
// **Always** return false if you are unsure if the props have changed
std::rc::Rc::ptr_eq(&self.name, &other.name)
}
}

fn RcPartialEq(name: RcPartialEqProps) -> Element {
rsx! {
div {
"Hello {name.name:?}"
}
}
}

fn RcPartialEqParent() -> Element {
let name = use_signal(|| Rc::new(NonPartialEq));

rsx! {
RcPartialEq {
// Generally, RcPartialEq will rerun even if the value of name hasn't actually changed because the Rc will point to a different value
name: name()
}
}
}
// ANCHOR_END: permissive_partial_eq