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

fix: Fix indexmap compatibility for yew 0.21.0 (#3659) #3677

Conversation

ryo-ebata
Copy link

@ryo-ebata ryo-ebata commented Jul 7, 2024

Description

Fix conversion from IndexMap to IMap in IntoPropValue implementation

This PR addresses an error in the IntoPropValue implementation for IndexMap to IMap conversion. The original code used IMap::from(self), which failed because there's no direct From implementation for IMap from indexmap::IndexMap.

Changes made:

  • Replace IMap::from(self) with IMap::from_iter(self)

Rationale:

  • from_iter utilizes the FromIterator trait to construct an IMap from an iterator.
  • IndexMap implements IntoIterator, allowing it to be used with from_iter.
  • This approach avoids the need for a direct From implementation between IMap and IndexMap.

The change resolves the compilation error and provides a more flexible conversion method using existing trait implementations.


This change may not directly resolve the Issue, but at least it resolved a similar error that was occurring in my environment

I added yew to the dependencies package,

yew = { version = "0.21", features = ["csr"] }

I added yew to the dependencies package, and ran into the error in this Issue when I did a simple HTML build to see how it worked.

Here is the code of main.rs at that time.

use yew::prelude::*;

#[function_component(App)]
fn app() -> Html {
    html! {
        <h1>{ "Hello World" }</h1>
    }
}

fn main() {
    yew::Renderer::<App>::new().render(); }
}

addresses #3659

Checklist

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

Copy link

github-actions bot commented Jul 7, 2024

Benchmark - core

Yew Master

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.857 ns      │ 2.967 ns      │ 2.877 ns      │ 2.881 ns      │ 100     │ 1000000000

Pull Request

vnode           fastest       │ slowest       │ median        │ mean          │ samples │ iters
╰─ vnode_clone  2.774 ns      │ 2.856 ns      │ 2.796 ns      │ 2.799 ns      │ 100     │ 1000000000

Copy link

github-actions bot commented Jul 7, 2024

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

https://yew-rs-api--pr3677-fix-trait-bound-erro-jcsx603v.web.app

(expires Sun, 14 Jul 2024 08:41:18 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Copy link

github-actions bot commented Jul 7, 2024

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
async_clock 97.703 97.703 0 0.000%
boids 167.831 167.831 0 0.000%
communication_child_to_parent 90.178 90.178 0 0.000%
communication_grandchild_with_grandparent 102.408 102.408 0 0.000%
communication_grandparent_to_grandchild 97.839 97.839 0 0.000%
communication_parent_to_child 86.538 86.538 0 0.000%
contexts 101.996 101.996 0 0.000%
counter 83.181 83.181 0 0.000%
counter_functional 83.608 83.608 0 0.000%
dyn_create_destroy_apps 86.562 86.562 0 0.000%
file_upload 97.579 97.579 0 0.000%
function_memory_game 163.902 163.902 0 0.000%
function_router 338.020 338.020 0 0.000%
function_todomvc 157.894 157.894 0 0.000%
futures 225.779 225.779 0 0.000%
game_of_life 105.289 105.289 0 0.000%
immutable 185.586 185.586 0 0.000%
inner_html 77.536 77.536 0 0.000%
js_callback 105.768 105.768 0 0.000%
keyed_list 195.335 195.335 0 0.000%
mount_point 80.514 80.514 0 0.000%
nested_list 111.237 111.237 0 0.000%
node_refs 87.854 87.854 0 0.000%
password_strength 1711.994 1711.994 0 0.000%
portals 90.778 90.778 0 0.000%
router 308.682 308.682 0 0.000%
simple_ssr 137.714 137.714 0 0.000%
ssr_router 375.264 375.264 0 0.000%
suspense 112.045 112.045 0 0.000%
timer 86.127 86.127 0 0.000%
timer_functional 94.845 94.845 0 0.000%
todomvc 139.053 139.053 0 0.000%
two_apps 83.210 83.210 0 0.000%
web_worker_fib 131.468 131.468 0 0.000%
web_worker_prime 181.964 181.964 0 0.000%
webgl 80.237 80.237 0 0.000%

✅ None of the examples has changed their size significantly.

Copy link

github-actions bot commented Jul 7, 2024

Benchmark - SSR

Yew Master

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 309.012 311.972 309.472 0.895
Hello World 10 491.130 508.596 497.062 6.699
Function Router 10 1633.230 1688.723 1656.293 16.782
Concurrent Task 10 1005.421 1006.487 1006.039 0.378
Many Providers 10 1106.762 1165.027 1125.556 18.784

Pull Request

Benchmark Round Min (ms) Max (ms) Mean (ms) Standard Deviation
Baseline 10 308.957 311.280 309.344 0.694
Hello World 10 494.388 510.971 501.460 5.487
Function Router 10 1667.320 1691.527 1674.082 7.509
Concurrent Task 10 1006.023 1007.177 1006.535 0.416
Many Providers 10 1094.715 1133.608 1108.498 11.612

@ryo-ebata
Copy link
Author

Incidentally, CI is making errors in areas unrelated to my changes, but I have not done anything special about this since it is not causally related to this issue.

@ryo-ebata ryo-ebata marked this pull request as ready for review July 7, 2024 09:01
@its-the-shrimp
Copy link
Contributor

This error is a result of implicit-clone, a daughter crate of Yew, trying to support both indexmap v1 & v2
See #3659 for more discussions on this, and implicit-clone/#51 that will prevent this from happening in future Yew versions

@ryo-ebata
Copy link
Author

@its-the-shrimp
thanks, I understood and close this.

@ryo-ebata ryo-ebata closed this Jul 13, 2024
@ryo-ebata ryo-ebata deleted the fix-trait-bound-error-for-indexmap-3659 branch July 13, 2024 06:32
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.

2 participants