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

Remove Sablono dependency, do templating internally #198

Closed
tonsky opened this issue Apr 7, 2020 · 0 comments · Fixed by #205
Closed

Remove Sablono dependency, do templating internally #198

tonsky opened this issue Apr 7, 2020 · 0 comments · Fixed by #205

Comments

@tonsky
Copy link
Owner

tonsky commented Apr 7, 2020

Rum batches all updates and applies them all at once on requestAnimationFrame.

This doesn’t work well with DOM/React input fields and controlled values: they need re-render synchronously, otherwise you’ll see junk while typing in input field, selection will be broken, etc.

Sablono has special code that wraps every input in a wrapper that does that synchronous update directly in place, without waiting for next frame or anything. Problem is, this code adds complexity, is not entirely transparent, and doesn’t work very well. We’ve worked with r0man tried to fix it a few times (r0man/sablono#129, r0man/sablono#127, r0man/sablono#123) but ultimately we didn’t succeeded. In fact, I believe it is still broken somehow in 0.8.6, and that’s the reason why Rum stays on Sablono 0.8.1 instead.

Now, it all can be solved much easier if Sablono could call Rum re-render from input handlers. You wouldn’t need wrappers, I believe. Problem is, Sablono doesn’t know anything about Rum.

Solution I propose: remove Sablono dependency and implement templating entirely inside Rum. That would allow for much simpler and (hopefully) finally working inputs code.

Another benefit would be that server-rendering code would more closely match client-rendering code with regards to e.g. nil handling etc (r0man/sablono#132).

It will also allow us to fix some issues that are pending in Sablono as it seems not to be actively maintained, e.g. r0man/sablono#201

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 a pull request may close this issue.

1 participant