Add support for raw HTML#787
Add support for raw HTML#787flip111 wants to merge 6 commits intopurescript-halogen:masterpurescript-halogen/purescript-halogen:masterfrom
Conversation
src/Halogen/VDom/Driver.js
Outdated
| var template = document.createElement('template'); | ||
| html = html.trim(); // Never return a text node of whitespace as the result | ||
| template.innerHTML = html; | ||
| var newElement = template.content.firstChild; |
There was a problem hiding this comment.
Does this mean it only supports a single HTML element? That should at least be documented somewhere. Maybe with support for fragments it could be improved to capture all children.
There was a problem hiding this comment.
I hadn't considered that .. but good point. I guess it does mean that.
It doesn't necessarily need to be like this but inserting multiple elements would need to happen at the parent of the current element. Which i think would make the code unnecessary more complicated and might not even be possible (what to do if the raw-html was the top halogen element).
Also i think it's unwanted to enter multiple nodes in one location. Because you can just put a node around it. i.e. <b>node1</b><b>node2</b> into <div><b>node1</b><b>node2</b></div>. Also another option the user can do [ H.rawHTML "<b>node1</b>", H.rawHTML "<b>node2</b>" ] if the wrapper node is really unwanted
What could be done is count the amount of children and if it's more than 1 to show a console warning on runtime.
Yes and documentation.
* Console error on passing not exactly 1 element to rawHTML function
|
How can i compare the html string How can i make this more efficient ? Like comparing the new html string with the old one and not render in case they are the same ? |
The original implementation used Nothing as the widget state for RawHTML nodes. This meant the patch function couldn't access the old DOM node to remove it, and the halt function couldn't clean up. Every re-render created a new element via insertChildIx without removing the old one, causing duplicate nodes to accumulate. Fix by replacing the Maybe-based WidgetState with a proper sum type (ThunkWidget | RawHTMLWidget | NoWidget) so each widget variant tracks the state it needs for correct patching and cleanup. Also fixes JS FFI bugs: - outerHtml -> outerHTML (wrong casing, returned undefined) - string concat . -> + (PHP syntax, not JS) - console.error -> throw Error (fail loud on invalid input) - Replace outerHTML setter approach with replaceWith() to avoid stale node references Merge upstream/master (6 commits, no conflicts). Expand raw-html example to cover dynamic, conditional, and mixed content scenarios.
|
Hey, I've come back to this after a long time. The branch is now merged up with upstream master and the implementation is reworked. The original version had a bug where old DOM nodes were never removed during re-renders. Every time the component updated, a new element was inserted but the previous one stayed in the DOM. This caused duplicate nodes to pile up and all kinds of weird browser behavior. The root cause was using There were also some JS bugs. The example is expanded to cover dynamic updates, conditional rendering, mixed content, and growing lists. I verified in the browser that all scenarios work correctly including rapid toggling and rapid state updates. Regarding the open questions from before: I'm keeping Happy to address any review feedback. |
tests pass
most examples are working. The ones that don't i don't think it's because of this PR but didn't look into it
comes with example of usage
Addresses #324
HalogenWidgetimo because the name doesn't cover the extra constructor. This would be a breaking change, while currently what is added i don't think it breaks anything so it could be updated in minor version of halogen library.rawHTMLjust tohtmlorraw?Review appreciated.