maciejhirsz
commented
1 year ago Your loop doesn't work since loops aren't iterators.
Note that for in { for (0..data.len()).map(...) } is not a for loop, it's a keyword that turns an iterator into a view.
If you need to branch inside an iterator, do it inside the map closure:
for (0..data.len()).map(move |index| {
if condition {
Branch2::A(view! {})
} else {
Branch2::B(view! {})
}
})Also couple notes:
Use iterators
Idiomatic way to iterate over entries in a vec or slice is:
/// loop:
for entry in data.iter() { ... }
/// mapping iterator:
data.iter().map(|entry| { ... })If you need the entry and index use enumerate:
/// loop:
for (index, entry) in data.iter().enumerate() { ... }
/// mapping iterator:
data.iter().enumerate().map(|(index, entry)| { ... })Not only is this cleaner and less bug prone, it can also potentially compile to faster code since it guarantees that bounds checks are unnecessary.
Don't do unnecessary clones
I know people do these a lot when they learn the language, but after using the language for a long time it hurts my soul when I see an unnecessary clone.
// this makes a new `String` which allocates and copies content:
data[index].1.clone()
// instead this just grabs a `&String` reference:
&data[index].1
// even better, this grabs a `&str` reference, which has less pointer indirection:
data[index].1.as_str()
// and of course if you switch to using iterator over data it would be:
entry.1.as_str()You don't need to turn integers to strings in Kobold
This is related to the above note, Kobold automatically stringifies all attributes that aren't known to it, so:
// this:
data_index={ index.to_string() }
// is just a very inefficient way of doing this:
data_index={ index }Note: using an undefined attribute like this will become a compile error soon, though I now realize I need to add support for data attributes.
Instead of passing data via attributes, just capture them in closures
Instead of this:
<input.edit
// ...
data_index={ index.to_string() }
onchange={
state.bind(move |state, e: Event<InputElement>| {
if let Some(data_index) = e.target().get_attribute("data_index") {
let index: usize = data_index.parse::<usize>().unwrap();
state.details.table.rows[0][index] = Text::Owned(e.target().value().into());
state.entry[index].editing = false;
}
})
}
// ...Just do this:
<input.edit
// ...
onchange={
// The `move` keyword in front of a closure means that captured values
// are moved (or copied if they implement `Copy`, which `usize` does)
// inside the closure.
//
// Event handlers need to live for a `'static` lifetime, or in simpler
// terms they need to "own" their content. If you didn't put the `move`
// here the `index` inside would be borrowed (`&usize` instead of `usize`),
// and the code would fail to compile.
state.bind(move |state, e: Event<InputElement>| {
// Since there is no `index` passed in as a parameter in this closure,
// it's moved (copied) inside it from parent scope.
state.details.table.rows[0][index] = Text::Owned(e.target().value().into());
state.entry[index].editing = false;
})
}
// ...Edit:
If you are curious about performance of closures and how this all works internally on re-renders in Kobold: this onchange closure will be two "words" in size (one for bound state, one for index). In wasm32 that just means 8 bytes (2 * 32 bits). The actual JavaScript callback is created once on the initial render. Subsequent renders just move the new closure into the memory allocated for the previous closure:
There is no diffing as this doesn't interact with JS/DOM side at all, and the difference between having to copy 4 bytes (just the state reference) and 8 bytes (state + index) is so small it's immaterial.
There is going to be a small performance difference between bound closures and ones that don't capture anything, namely this handler:
onmouseover={|e: MouseEvent<InputElement>| e.target().focus()}Is zero-sized (it's a plain/unbound closure that doesn't capture anything). Internally it's still put into a Box, but Rust is smart enough to know that it doesn't need to allocate memory for something that takes no memory to represent to begin with, so the initial render is going to be faster. The memory use isn't a problem, it's just that creating allocations (be it a Box<T>, a Vec<T>, or a String) is relatively "expensive".
If you look at the update here again:
The self takes no memory at all (is 0-bytes) == there is nothing to move == this compiles to nothing at all.
ltfschoen
I've created this
forloop here, code snippet:It iterates through the number of properties in
datavariable. for context,datais a vector of tuplesVec<(String, String)>, where each tuple contains the property key and property value that was converted from theDetailsstruct here from default state values or from loading the details.csv file (also see unit test of how that was done here)Then I want to do the conditional branching from the docs. So to make the code clearer I've created a: *
DetailEditingcomponent (which is to only render ifstate.entry[index].editing == trueDetailViewcomponent (which renders they're not editing the input field with thatindex)The problems that I'm facing is: 1) It gives error:
click to show error
```bash error[E0308]: mismatched types --> examples/invoice/src/main.rs:389:9 | 311 | fn DetailView(index: usize, data: Vec<(String, String)>, state: &Hookdata.len()equals10, since there are 10 columns of data being loaded from the details.csv file. And so in this case I think I need to use branching with#[component]and Branch9 instead of Branch2, but the max branch I can use isBranch9, and even if I only had 9 columns of data, and usedBranch9, I then need to use each of A,B,C,D,E,F,G,H,I for each of the 9 times that I loop through withfor index in 0..data.len() {(i.e.Branch9::A,Branch9::B, ...,Branch9::I), so I need to fit 9 branches in 2 blocks, since I only have anifand anelseblock to put them in... So I could remove the outer loopfor index in 0..data.len() {and just manually add a branch for every possibility like the following, but that seems like an extreme measure, and again, I'm not sure whether I need Branch9 or if Branch2 is actually sufficient in this scenerio and I'm just using it wrong. And to fully cater for indexes 0 to 3 I have to use Branch8, since if I use Branch9 I can only cater for an ::I branchstate.entry[4].editing == truebut not a ::J branch to cater forstate.entry[4].editing == false, but I can use ::J to cater for theelseat the end.Whilst that compiles, it only renders one of the input fields instead of all of them.
So then I reverted my code to the way it was before doing all this: But that only works when I change
if state.entry[index].editing == true {andlet editing = class!("editing" if state.entry[index].editing);to have hard-coded values ofindexlike0, but then it triggers out of bounds errors as expected since I'm not sure how to move the if statement inside theview!Sorry for the convoluted post but any tips on whether it's possible to have an
ifstatement insidefor (0..data.len()).map(move |index|, and whether it's bad practice to have a loop and the condition outside a Branch like:I'm going to revert back to using
CellandHeadin the interim.