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

Refactor vlist to use less macros and unsafe #2195

Merged
merged 1 commit into from
Nov 27, 2021

Conversation

WorldSEnder
Copy link
Member

@WorldSEnder WorldSEnder commented Nov 25, 2021

Description

Refactors yew::virtual_dom::vlist to be more idiomatic rust.

An ElementWriter struct keeps track of the place where to insert elements, instead of the previous apply! macro.
The Vec::drain API is used to consume ancestors. This removes a previous worry where panic doesn't drop the remaining elements.

No changes to algorithmic nor runtime behaviour in this PR.

Additional small fix, cause I noticed a warning: author -> authors in web_worker_fib.

Checklist

  • I have run cargo make pr-flow
  • I have reviewed my own code

An ElementWriter struct keeps track of the place where to insert elements,
instead of the previous apply! macro.
The Vec::drain api is used consume ancestors. This removes a panic unsafety:
the previous impl forgets to drop elements

Small fix: author -> authors in web_worker_fib
@ranile ranile requested review from mc1098 and siku2 November 25, 2021 11:21
@ranile
Copy link
Member

ranile commented Nov 25, 2021

This is something I haven't really touched much so it'd be better if someone with more experience can take a look at this

@voidpumpkin voidpumpkin added the A-yew Area: The main yew crate label Nov 25, 2021
@mc1098 mc1098 merged commit 3707ea5 into yewstack:master Nov 27, 2021
@WorldSEnder WorldSEnder deleted the refactor-vlist branch November 27, 2021 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-yew Area: The main yew crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants