Choo v5 / v6 onload handlers on root node not called when navigating

[mantoni]

Steps to reproduce behavior

Create two pages:

• /foo with root <div>
• /bar with root <div onload=${myHandler}>

Expected behavior

Navigating from /foo to /bar triggers the onload handler

Actual behavior

The onload handler is not triggered upon navigation. It is triggered when loading the /bar page initially though.

Workaround

Attaching the onload handler to a child node fixes the issue.

The workaround is simple enough to apply. If this is tricky to solve, I think an assertion would also do.

[bengourley]

If I understand it correctly, this is a typical nanomorph/morphdom issue… when you navigate between routes, the diffing algorithm compares…

before: <div></div>
after: <div onload=${fn}></div>

It sees that the element that exists is already a <div> so doesn't need to replace it, just add an attribute. Since the semantics of onload are "when the element is added to the DOM" and this element is already in the DOM, it correctly (but confusingly) doesn't fire.

I don't know what the best approach for your problem is this but I hope this helps explain why it doesn't work!

[mantoni]

@bengourley Thanks for elaborating. I'm aware of this behavior, but still crashed into this and got confused. It should somehow be handled or highlighted by Choo, I think.

[josephluck]

If you set unique IDs on the nodes, it'll treat it as a new node and onload will fire correctly On 9 May 2017 09:57, Maximilian Antoni <[email protected]> wrote:@bengourley Thanks for elaborating. I'm aware of this behavior, but still crashed into this and got confused. It should somehow be handled or highlighted by Choo, I think. —You are receiving this because you are subscribed to this thread.Reply to this email directly, view it on GitHub, or mute the thread.

[mantoni]

No, setting unique IDs on the nodes does not help. Actually, the onload handler generates dynamic data properties that are already unique (as long as the called handler function is different).

Here is a minimal Choo app to illustrate the issue:

var html = require('choo/html')
var choo = require('choo')

var app = choo()

function onloadA() {
  console.log('onload A')
}

function onloadB() {
  console.log('onload B')
}

app.route('/a', function (state, emit) {
  return html`<div onload=${onloadA} id="a"><a href="/b">Go to B</a></div>`
})
app.route('/b', function (state, emit) {
  return html`<div onload=${onloadB} id="b"><a href="/a">Go to A</a></div>`
})

document.body.appendChild(app.start())

Save as issue.js, run with budo issue.js --pushstate and open http://localhost:9966/a.

[mantoni]

Actually, if you look at the DOM changes when navigating between /a and /b, you can observe that the id property is changed on the root node while the data-onloadHASH property is not. Only onload A makes it to the console on first load.

[nilliams]

I just hit this too and can confirm the issue.

Edit: Speculated about using data-onloadid as a workaround, but deleted as I realised I have no idea of the intended use for that.

[yoshuawuyts]

Think this is related - we need to change this (: choojs/nanomorph#63

…

On Mon, May 15, 2017 at 1:51 PM Nick Williams ***@***.***> wrote: I just hit this too and can confirm the issue. Looking at the nanomorph code I spotted another workaround that seems to work, setting unique values for data-onloadid on your root elements, which is slightly nicer than setting it on a child element (as that can require creating one just for the purpose). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#490 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACWlegP8kaEm_tFVSt9eae9BWFexptn0ks5r6DxGgaJpZM4NU-m6> .

[yoshuawuyts]

choojs/nanomorph#63 will land in choo v6 ✨

[mantoni]

I had another look with Choo 6.0 and Choo 6.5. Now none of the onload handlers is fired anymore with the example posted above. Previously, at least the initial onload handler was fired. Is this intentional and root node onload is not supported at all anymore?

[yoshuawuyts]

@mantoni yeah, we removed it entirely — this was a breaking change in v6.0.0, but needed to happen because using onload with DOM diffing doesn't work. We recommend using nanocomponent instead!

Does that make sense?

[mantoni]

Makes sense. It's at least consistent now. However, I'd personally think this is a common thing to try when playing with Choo. How about adding an assertion?

[yoshuawuyts]

What do you mean by assertion? Where would it go?

…

On Mon, Oct 23, 2017 at 1:27 PM Maximilian Antoni ***@***.***> wrote: Makes sense. It's at least consistent now. However, I'd personally think this is a common thing to try when playing with Choo. How about adding an assertion? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#490 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACWleptRE5azZh0lmmo2ryKEQf9pfubAks5svHgrgaJpZM4NU-m6> .

[mantoni]

I‘m mean that Choo should throw if you’re using onload on a root element. Because it’s working otherwise, just not on the root.

[yoshuawuyts]

@mantoni please check you're running the latest bel; I made sure to remove it in a major version a couple of months ago. There's valid use cases for an onload attribute that acts differently from the on-load package, so throwing when it's used would probably be a conflicting (breaking even) change. Does this make sense?

[mantoni]

@yoshuawuyts Thanks for the pointer. Took me a while to find out why my code is still working 😅

Turns out I'm always going through yo-yoify and never hit bel directly. The onload feature was not removed from yo-yoify (yet).

Closing issue since it's by design. However, I'm not sure how I'm supposed to migrate my codebase once onload stops working. I depend heavily on this feature and using the on-load library is not as convenient 🤔

[yoshuawuyts]

Short reply as it's late, but check out: - https://github.com/choojs/nanocomponent - #593

…

On Mon, Oct 30, 2017 at 8:44 PM Maximilian Antoni ***@***.***> wrote: @yoshuawuyts <https://github.com/yoshuawuyts> Thanks for the pointer. Took me a while to find out why my code is still working 😅 Turns out I'm always going through yo-yoify and never hit bel directly. The onload feature was not removed from yo-yoify (yet). Closing issue since it's by design. However, I'm not sure how I'm supposed to migrate my codebase once onload stops working. I depend heavily on this feature and using the on-load library is not as convenient 🤔 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#490 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACWlelyj91xuEToGZdhwotz2AbJ-XzdSks5sxicWgaJpZM4NU-m6> .