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

Add option to reuse pre-rendered markup & to reuse DOM nodes #423

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ctizen
Copy link

@ctizen ctizen commented Jun 1, 2016

Hello,

I've taken 0a's changes from #395 and #396, slightly refactored them to comply with current codebase and added some more things, such as possibility to reuse already created nodes to minimize memory usage.
Also relates to #190

I still have a TODO to resolve (see question in test/app/thunk.js), any advice is appreciated. Also I'm not sure if all of these changes should be in the mainline or it's better to make them as a plugin.

Bench for uibench ( https://localvoid.github.io/uibench/ ) may be run with this custom link: https://heilage-nsk.github.io/uibench-deku/
Headless tests are passing, but for some reason Electron fails to start on my system, so I cannot check it :( I'd be glad if someone would try to run tests on Electron and share the results.

Oleg Klimenko added 9 commits June 1, 2016 17:13
…n to do checksum comparison)

This reverts commit b631711.

Add tests & doc for string-eq-based self-healing (i.e. autoFix)

Add string-eq-based self-healing

fix basic example to adapt new createApp behavoir

add test for pre-render HTML event handling

Modify setAttribute to accept an option parameter

Reconstruct createElement so the side effects are now accessible

Implement event handler registration for pre rendered HTML

Rebase fix
Add failing rerender test

Add path-based virtualdom restoration on client-side

libs cleanup
get rid of uids on server

Remove generator to avoid regenerator runtime
@anthonyshort
Copy link
Owner

Thanks for the PR @heilage-nsk! I'm going to have a read through this soon and give you some feedback.

@Chaptykov
Copy link

+1

2 similar comments
@alexHlebnikov
Copy link

+1

@ArtZanko
Copy link

+1

@anthonyshort
Copy link
Owner

Looks good to me! The pooling seems like it could exist in an another module rather than living inside Deku, but we can move this out at a later stage. Nice work!

For the thunk, having nested thunks with the same path could break anything using the path to store state, so we might need to fix that before we can merge it in.

@ctizen
Copy link
Author

ctizen commented Jul 11, 2016

@anthonyshort
Sorry for it took me so long. I've just pushed a fix for paths in thunks.

@todesignandconquer
Copy link

+1

@markuplab
Copy link

+1? @anthonyshort maybe merge it?

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.

7 participants