Hyperapp: Children appended to script element when using root: document.body
Hi,
When I specify root: document.body I get children of my component appended to the script tag:
Code
View
const { h } = require('hyperapp')
const hyperx = require('hyperx')
const html = hyperx(h)
const ScheduleEvent = (event) => html`
<li class="schedule-event">
<span class="event">${event.title}</span>
<span class="hours">
<time>${event.start}</time> -
<time>${event.end}</time>
</span>
</li>
`
const ScheduleDay = (day) => html`
<li class="schedule-day">
<span class="day"></span>
<ul class="events">
${day.events.map(event => ScheduleEvent(event))}
</ul>
</li>
`
const ScheduleWidget = (schedule) => html`
<ul class="schedule-widget">
${schedule.days.map(day => ScheduleDay(day))}
</ul>
`
module.exports = ScheduleWidget
Index
const { app } = require('hyperapp')
const model = require('./lib/model')
const view = require('./lib/view')
const reducers = require('./lib/reducers')
const subscriptions = require('./lib/subscriptions')
app({ model, view, reducers, subscriptions, root: document.body })
ngryman
All 25 comments
@ngryman Yup, this is not in the docs, but hyperapp does not render itself in the body as that would create edge complexity. You can either use a regular element inside document.body.
root: document.getElementById("my-element")
or provide none and hyperapp will append a <div> to document.body for you and render its virtual tree in it.
jorgebucaran
on 5 Mar 2017
@jbucaran Ok thanks! It would be worth mentioning in the docs. I can provide a PR if you want, but my english is not ... ✨
ngryman
on 5 Mar 2017
@ngryman That's okay, the docs are not in the repo but in the wiki, which is open / public so you can change them anytime, no need to ask for permission, just do it! :)
Btw, even better would be to fix patch so that hyperapp can render a tree in document.body. I remember I tried once and failed, but maybe it's time to try again.
jorgebucaran
on 5 Mar 2017
Well I added this test case which passes:
it("appends view to document body", () => {
app({
root: document.body,
view: _ => h("ul", {}, h("li", {}, "bar"))
})
expectHTMLToBe(`
<ul>
<li>
bar
</li>
</ul>
`)
})
I can't reproduce in your test environment. Could it be related to the script element in some way?
ngryman
on 5 Mar 2017
@jbucaran If I add other nodes to document.body before calling app, it always insert the children in the first element. It must be related somehow to the fact that you expect the root to only have one child (ul here) and so append other children in the first root child that end up being script here.
I still don't know the details of your implementation, but if it gives you some clue, then 🎉
ngryman
on 5 Mar 2017
@ngryman I'm quite sure it's because of the <script> element. You can just move it to the <head> and everything should work.
FlorianWendelborn
on 6 Mar 2017
@dodekeract I will! I know document.body is a dangerous potential root element as other non app-related elements can live there. Still, for simple use cases like mine it's perfectly valid. If I need another tag like a svg store for example, it will behaves the same. It seems to be a really tiny bug...
@jbucaran @dodekeract Do you think you could take a quick look at what's happening here? Meanwhile I will update the docs mentioning that the user should avoid using document.body.
ngryman
on 6 Mar 2017
Whoa, now I can't repro this anymore? Can you help @ngryman? 🙏
jorgebucaran
on 7 Mar 2017
jorgebucaran
on 7 Mar 2017
@jbucaran Interesting... Let me see if I can create a gomix that reproduce the issue.
ngryman
on 7 Mar 2017
@ngryman Nevermind, I wasn't able to repro, but basically using document.body causes hyperapp to self-implode when the model if subsequently updated.
jorgebucaran
on 7 Mar 2017
Here is the reproduced issue (with version 0.6.0): https://gomix.com/#!/join/fefd85c2-0d11-405e-96c7-e9bc21212eb6. With 0.7.x it crashes.
ngryman
on 7 Mar 2017
@jbucaran I have some time today, I can try to see if I come up with something, but first I have to understand the actual code 🤓
ngryman
on 7 Mar 2017
I'd say this issue should be renamed "breaks when root is not empty".
FlorianWendelborn
on 7 Mar 2017
Let's clarify, the problem is hyperapp crashes or behaves unexpectedly if root is set to ~document.body~ any non-empty element. ~and document.body already has children in it.~
jorgebucaran
on 7 Mar 2017
@jbucaran does this only happen on body? I'd be surprised if the root being not empty doesn't cause the whole bug.
FlorianWendelborn
on 7 Mar 2017
@dodekeract You're right, same behavior given any non-empty root: https://gomix.com/#!/join/fefd85c2-0d11-405e-96c7-e9bc21212eb6.
ngryman
on 7 Mar 2017
@jbucaran It seems to be related to subscriptions or mutating the model.
If you feed the model directly or put static markup the issue does not appear. But if you feed the model with subscriptions or mutate it by some other means I guess, then the issue appears. If this can help...
EDIT: tldr, first patch works, subsequent won't.
ngryman
on 7 Mar 2017
@jbucaran Couldn't it be related to the hardcoded 0 at app.js:L:116 patch(root, node, node = view(model, actions), 0)?
ngryman
on 7 Mar 2017
@jbucaran I guess a simple patch would be:
patch(
root,
node,
node = view(model, actions),
Array.prototype.indexOf.call(root.childNodes, node)
)
@ngryman Awesome, can you PR?
jorgebucaran
on 7 Mar 2017
@jbucaran Working on it.
ngryman
on 7 Mar 2017
Closing in favor of ~#128~ #140.
jorgebucaran
on 7 Mar 2017
@jbucaran You just closed #128 in favor of #128.
FlorianWendelborn
on 7 Mar 2017
Fixed. 🤣
jorgebucaran
on 7 Mar 2017
Related issues
zaceno
·
3Comments
jorgebucaran
·
4Comments
ghost
·
3Comments
jacobtipp
·
3Comments
zhaotoday
·
3Comments