Skip to content

concise diagnostics for missing props #3873

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

Merged
merged 4 commits into from
Jul 9, 2025

Conversation

Madoshakalaka
Copy link
Member

@Madoshakalaka Madoshakalaka commented Jul 6, 2025

Description

Leveraging the newly stablized #[diagnostics] attributes, we can massively clean up diagnostics for missing props.

#[diagnostics::on_unimplemented] is stablized in rust 1.78, May, 2024
#[diagnostics::do_not_recommend] is stablized in rust 1.85.0, February 20, 2025

This PR raises the MSRV from 1.76 to 1.78.
the use of #[diagnostics::do_not_recommend] seems simply ignored in versions before 1.85.

consider the following example:

use yew::prelude::*;

#[function_component]
pub fn App() -> Html {
    html! {
        <Foo bar={"bar".to_string()} />
    }
}

#[derive(Properties, PartialEq, Clone)]
pub struct FooProps {
    pub bar: String,
    pub baz: u32,
}

#[function_component]
pub fn Foo(props: &FooProps) -> Html {
    html! {}
}

old diagnostics:
image

new diagnostics (1.78):

image

new diagnostics (1.85)

image

Checklist

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

Copy link

github-actions bot commented Jul 6, 2025

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

https://yew-rs--pr3873-missing-props-diagno-8yfivone.web.app

(expires Tue, 15 Jul 2025 10:19:10 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@@ -1,4 +1,3 @@
#![no_implicit_prelude]
Copy link
Member Author

@Madoshakalaka Madoshakalaka Jul 6, 2025

Choose a reason for hiding this comment

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

I removed these from the tests that have #[derive(Properties)] because it prevents #[diagnostics] from being used. I have no clue how to qualify diagnostics with no_implicit_prelude. If anybody can help it's very appreciated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm.. I think it's impossible to use no_implicit_prelude together with diagnostic

https://doc.rust-lang.org/reference/attributes.html#r-attributes.tool.prelude

Tool attributes are not available if the no_implicit_prelude attribute is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think no_implicit_prelude was introduced to improve hygiene, it ensures our macros don't accidentally collide with rust's built-in ones, which could cause false passes in macro tests.

Sadly there seems to be no way around it for now and diagnostic improvements seem worth it.

Copy link

github-actions bot commented Jul 6, 2025

Benchmark - core

Yew Master

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.435 ns      │ 2.763 ns      │ 2.436 ns      │ 2.441 ns      │ 100     │ 1000000000

Pull Request

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.402 ns      │ 3.765 ns      │ 2.407 ns      │ 2.78 ns       │ 100     │ 1000000000

Copy link

github-actions bot commented Jul 6, 2025

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 290.768 296.639 292.137 2.372
Hello World 10 476.648 488.333 478.717 3.469
Function Router 10 1577.379 1588.320 1583.150 3.622
Concurrent Task 10 1005.869 1007.458 1006.657 0.439
Many Providers 10 1084.245 1127.922 1099.981 11.930

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 290.845 291.256 290.978 0.137
Hello World 10 506.985 528.298 515.581 7.987
Function Router 10 1587.290 1618.046 1603.822 11.483
Concurrent Task 10 1005.423 1007.097 1006.432 0.454
Many Providers 10 1100.311 1139.490 1117.065 12.264

Copy link

github-actions bot commented Jul 6, 2025

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 100.058 100.058 0 0.000%
boids 168.688 168.688 0 0.000%
communication_child_to_parent 92.301 92.301 0 0.000%
communication_grandchild_with_grandparent 103.483 103.483 0 0.000%
communication_grandparent_to_grandchild 98.490 98.490 0 0.000%
communication_parent_to_child 88.175 88.175 0 0.000%
contexts 104.571 104.571 0 0.000%
counter 84.886 84.886 0 0.000%
counter_functional 85.301 85.301 0 0.000%
dyn_create_destroy_apps 87.811 87.811 0 0.000%
file_upload 99.064 99.064 0 0.000%
function_memory_game 170.675 170.675 0 0.000%
function_router 338.952 338.952 0 0.000%
function_todomvc 163.947 163.947 0 0.000%
futures 237.054 237.054 0 0.000%
game_of_life 104.979 104.979 0 0.000%
immutable 195.396 195.396 0 0.000%
inner_html 80.556 80.556 0 0.000%
js_callback 108.041 108.041 0 0.000%
keyed_list 196.035 196.035 0 0.000%
mount_point 83.864 83.864 0 0.000%
nested_list 113.187 113.187 0 0.000%
node_refs 91.187 91.187 0 0.000%
password_strength 1786.093 1786.093 0 0.000%
portals 93.140 93.140 0 0.000%
router 307.746 307.746 0 0.000%
suspense 112.106 112.106 0 0.000%
timer 88.915 88.915 0 0.000%
timer_functional 95.153 95.153 0 0.000%
todomvc 143.958 143.958 0 0.000%
two_apps 86.250 86.250 0 0.000%
web_worker_fib 135.254 135.254 0 0.000%
web_worker_prime 185.647 185.647 0 0.000%
webgl 83.479 83.479 0 0.000%

✅ None of the examples has changed their size significantly.

@Madoshakalaka Madoshakalaka force-pushed the missing-props-diagnostics branch from 9206406 to ac43025 Compare July 6, 2025 08:19
@Madoshakalaka Madoshakalaka marked this pull request as ready for review July 6, 2025 19:20
github-actions[bot]
github-actions bot previously approved these changes Jul 6, 2025
github-actions[bot]
github-actions bot previously approved these changes Jul 6, 2025
github-actions[bot]
github-actions bot previously approved these changes Jul 8, 2025
@Madoshakalaka Madoshakalaka merged commit b4d083e into master Jul 9, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant