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

Avoid clippy::let-underscore-drop #2952

Merged
merged 1 commit into from Nov 8, 2022

Conversation

cschramm
Copy link
Contributor

@cschramm cschramm commented Nov 7, 2022

Description

The following main.rs replicates the clippy warning:

use yew::prelude::*;

struct Props {
    droppable: Vec<()>,
}

fn component(props: &Props) -> Html {
    let props = Props { droppable: Vec::new() };
    html! { <Component ..props /> }
}

fn main() {}

If I'm not mistaken this happens when using the .. on any Properties with a field that implements Drop.

Checklist

  • I have reviewed my own code
  • I have added tests

@WorldSEnder
Copy link
Member

Correct me if I'm wrong: Is this because clippy thinks let _ = #props_ident.#label; moves the field out of the prop struct - even though the method is never called?

If my assumption is correct, then perhaps instead of working around the mechanism by which clippy lints this (binding to _some_name instead of _), we could also use let _ = &#props_ident.#label; to avoid the move?

The following main.rs replicates the clippy warning:

```
use yew::prelude::*;

struct Props {
    droppable: Vec<()>,
}

fn component(props: &Props) -> Html {
    let props = Props { droppable: Vec::new() };
    html! { <Component ..props /> }
}

fn main() {}
```

If I'm not mistaken this happens when using the `..` on any `Properties` with a field that implements `Drop`.
@cschramm
Copy link
Contributor Author

cschramm commented Nov 7, 2022

Yes, you're right. That works just as well. 👌

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

Visit the preview URL for this PR (updated for commit 80f3a44):

https://yew-rs-api--pr2952-let-underscore-drop-g54b08uq.web.app

(expires Tue, 15 Nov 2022 15:13:56 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 357.538 410.253 375.299 16.923
Hello World 10 679.746 726.237 697.269 15.739
Function Router 10 2509.595 2751.933 2602.691 70.480
Concurrent Task 10 1009.518 1011.140 1010.194 0.567

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 353.273 392.905 366.720 13.427
Hello World 10 671.954 718.539 689.391 15.807
Function Router 10 2539.653 2636.505 2589.640 27.559
Concurrent Task 10 1008.583 1012.370 1009.908 1.072

@github-actions
Copy link

github-actions bot commented Nov 8, 2022

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 106.161 106.159 -0.002 -0.002%
boids 170.390 170.396 +0.006 +0.003%
communication_child_to_parent 90.032 90.030 -0.002 -0.002%
communication_grandchild_with_grandparent 104.881 104.880 -0.001 -0.001%
communication_grandparent_to_grandchild 100.746 100.742 -0.004 -0.004%
communication_parent_to_child 87.133 87.134 +0.001 +0.001%
contexts 107.362 107.354 -0.009 -0.008%
counter 85.049 85.050 +0.001 +0.001%
counter_functional 85.522 85.523 +0.001 +0.001%
dyn_create_destroy_apps 87.910 87.913 +0.003 +0.003%
file_upload 99.644 99.644 0 0.000%
function_memory_game 162.890 162.881 -0.009 -0.005%
function_router 347.179 347.186 +0.007 +0.002%
function_todomvc 157.651 157.645 -0.007 -0.004%
futures 221.941 221.942 +0.001 +0.000%
game_of_life 104.576 104.577 +0.001 +0.001%
immutable 180.987 180.974 -0.014 -0.008%
inner_html 81.910 81.911 +0.001 +0.001%
js_callback 110.881 110.882 +0.001 +0.001%
keyed_list 195.522 195.523 +0.001 +0.000%
mount_point 84.659 84.658 -0.001 -0.001%
nested_list 111.618 111.620 +0.002 +0.002%
node_refs 92.535 92.536 +0.001 +0.001%
password_strength 1546.184 1546.181 -0.003 -0.000%
portals 95.951 95.949 -0.002 -0.002%
router 316.991 316.996 +0.005 +0.002%
simple_ssr 151.584 151.582 -0.002 -0.001%
ssr_router 392.271 392.273 +0.003 +0.001%
suspense 108.703 108.697 -0.006 -0.005%
timer 87.941 87.939 -0.002 -0.002%
todomvc 138.959 138.962 +0.003 +0.002%
two_apps 85.714 85.713 -0.001 -0.001%
web_worker_fib 152.038 152.037 -0.001 -0.001%
webgl 84.352 84.351 -0.001 -0.001%

✅ None of the examples has changed their size significantly.

Copy link
Member

@hamza1311 hamza1311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@hamza1311 hamza1311 merged commit b649e9d into yewstack:master Nov 8, 2022
@hamza1311 hamza1311 added the A-yew-macro Area: The yew-macro crate label Nov 8, 2022
@cschramm cschramm deleted the let-underscore-drop branch November 8, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew-macro Area: The yew-macro crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants