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

Change access to VList children to a wrapper #2673

Merged
merged 4 commits into from
May 11, 2022

Conversation

WorldSEnder
Copy link
Member

Description

Fixes #2654

As a response to the confusion in #2654, restrict mutable access to the children of a VList to a wrapper that takes care of recomputing fully_keyed automatically.

Checklist

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

@WorldSEnder WorldSEnder changed the title Vlist children Change access to VList children to a wrapper May 10, 2022
@github-actions
Copy link

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

https://yew-rs-api--pr2673-vlist-children-nnp8i8rr.web.app

(expires Tue, 17 May 2022 07:59:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@github-actions
Copy link

Size Comparison

examples master (KB) pull request (KB) diff (KB) diff (%)
boids 172.770 172.770 0 0.000%
contexts 109.642 109.642 0 0.000%
counter 86.654 86.654 0 0.000%
counter_functional 87.305 87.305 0 0.000%
dyn_create_destroy_apps 89.800 89.800 0 0.000%
file_upload 102.620 102.620 0 0.000%
function_memory_game 167.354 167.354 0 0.000%
function_router 350.424 350.424 0 0.000%
function_todomvc 161.987 161.987 0 0.000%
futures 226.616 226.657 +0.041 +0.018%
game_of_life 107.539 107.539 0 0.000%
inner_html 83.687 83.687 0 0.000%
js_callback 112.869 112.869 0 0.000%
keyed_list 195.036 195.036 0 0.000%
mount_point 86.281 86.281 0 0.000%
nested_list 115.908 115.908 0 0.000%
node_refs 90.450 90.450 0 0.000%
password_strength 1539.232 1539.232 0 0.000%
portals 97.197 97.197 0 0.000%
router 319.468 319.468 0 0.000%
simple_ssr 494.013 494.013 0 0.000%
ssr_router 425.761 425.761 0 0.000%
suspense 110.522 110.522 0 0.000%
timer 89.361 89.361 0 0.000%
todomvc 143.053 143.053 0 0.000%
two_apps 87.286 87.286 0 0.000%
web_worker_fib 153.530 153.530 0 0.000%
webgl 87.375 87.375 0 0.000%

✅ None of the examples has changed their size significantly.

Copy link
Member

@ranile ranile left a comment

Choose a reason for hiding this comment

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

I believe it also fixes #1815

@WorldSEnder
Copy link
Member Author

WorldSEnder commented May 10, 2022

@hamza1311 I believe ChildrenRenderer is its own separate thing. But I also believe that could simply implement Deref and DerefMut by itself to solve the issue over there.

EDIT: will merge this tomorrow, when I had to time to consider #1815

@WorldSEnder WorldSEnder merged commit d7b43bb into yewstack:master May 11, 2022
@WorldSEnder WorldSEnder deleted the vlist-children branch May 11, 2022 10:37
WorldSEnder added a commit to WorldSEnder/yew that referenced this pull request May 19, 2022
VList again has a DerefMut implementation
the internal fully_keyed state now has an "indeterminate" variant
instead of being a bool, this recomputes it during reconciliation
WorldSEnder added a commit that referenced this pull request May 24, 2022
…st (#2692)

* partially undo #2673

VList again has a DerefMut implementation
the internal fully_keyed state now has an "indeterminate" variant
instead of being a bool, this recomputes it during reconciliation

* add Copy impl to FullyKeyedState
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loosing state since commit 78d4204a9a52a826bd2be4e7383d29e04233fb5b
2 participants