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

Split update and render operations into seprate tasks #1309

Merged
merged 3 commits into from
Jun 18, 2020

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Jun 14, 2020

Description

DOM updates are much more efficient when batched together but Yew does not do so when rendering components. This change moves the DOM updates to the "render" task (formerly "rendered"). This allows the scheduler to render to the DOM in quick succession since the virtual dom has already been created.

Related to #1153

Checklist:

  • I have run ./ci/run_stable_checks.sh
  • I have reviewed my own code
  • I have added tests – if this is a bug fix, these tests will fail if the bug is present (to stop it from cropping up again). If this is a feature, my tests prove that the feature works.

@jstarry
Copy link
Member Author

jstarry commented Jun 14, 2020

@siku2 any idea why the comment failed on the benchmark script?

@siku2
Copy link
Member

siku2 commented Jun 14, 2020

@siku2 any idea why the comment failed on the benchmark script?

No, not really... Could you try re-running the workflow?

@siku2
Copy link
Member

siku2 commented Jun 14, 2020

In my fork the comment is working (siku2#5) so I hope this was just a fluke...
But you broke the workflow anyway with your latest commit: yewstack/js-framework-benchmark@4a30ee2.
The following line needs to be updated because you changed the Cargo.toml file:

search="{ git = \"https://github.com/jstarry/yew\", branch = \"keyed-list\" }"

It should be the following now:
{ git = \"https://github.com/jstarry/yew\", branch = \"split-render\" }.

I honestly wasn't expecting you to change that line in the near future. I guess we need to come up with a better solution than a simple search and replace after all...

@jstarry
Copy link
Member Author

jstarry commented Jun 15, 2020

I honestly wasn't expecting you to change that line in the near future. I guess we need to come up with a better solution than a simple search and replace after all...

No worries, I kept it in the back of my mind too 😉 I just updated it here: yewstack/js-framework-benchmark@7bb7b65

@jstarry
Copy link
Member Author

jstarry commented Jun 15, 2020

@siku2 I think it's because of this:

Secrets are not passed to workflows that are triggered by a pull request from a fork

@jstarry
Copy link
Member Author

jstarry commented Jun 15, 2020

Maybe it can be triggered by adding a label to the PR?

@jstarry
Copy link
Member Author

jstarry commented Jun 15, 2020

Here's an issue: #1311

@jstarry
Copy link
Member Author

jstarry commented Jun 15, 2020

@mergify mergify bot merged commit 6450518 into yewstack:master Jun 18, 2020
teymour-aldridge pushed a commit to teymour-aldridge/yew that referenced this pull request Jun 20, 2020
* Split update and render operations into seprate tasks

* Fix functional components

* Rename internal functional component terms
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants