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 support for lydell’s fork of elm/virtual-dom #40

Open
wants to merge 1 commit into
base: lamdera-next
Choose a base branch
from

Conversation

lydell
Copy link
Contributor

@lydell lydell commented Feb 11, 2025

This makes Lamdera work with my fork of elm/virtual-dom. More information coming soon. No need to merge this yet.

Lamdera copies some functions from elm/virtual-dom, to make modifications to them. My fork of elm/virtual-dom also changes those functions. This PR copies those changes, and supports both the original version and my fork by if-ing on whether _VirtualDom_wrap exists (one of the new functions in my fork, arbitrary choice). This is for _VirtualDom_applyPatches, _VirtualDom_diff and _VirtualDom_applyPatches.

My fork also removes _VirtualDom_equalEvents, which Lamdera references. The solution is to just define the variable.

Finally, I added the data-elm attribute to <pre id="elm"></pre>. My fork only virtualizes elements with data-elm, for better compatibility with third-party scripts.

lamdera live assumes that initializing the Elm app removes that element, and displays a message in it otherwise:

compiler/extra/live.js

Lines 124 to 126 in 6415988

if (document.getElementById(elid)) {
document.getElementById(elid).innerText = 'This is a headless program, meaning there is nothing to show here.\n\nI started the program anyway though, and you can access it as `app` in the developer console.'
}

By adding data-elm to it, Elm will virtualize it, and then realize that it isn’t needed and remove it (just like before).

Note: I have only tested lamdera live. I have not tested a production upgrade between two app versions, but I’ve gone through the code and it looks like it should™️ work.

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.

1 participant