hyperapp-router
hyperapp-router copied to clipboard
jorgebucaran
Routed component's top node always destroyed
Please see this example: https://codepen.io/zaceno/pen/vRwERE?editors=0011
Each page ("A" and "B") has a plain p with no key as its top node. Notice when you navigate between "PageA" and "PageB" the console reports that ondestroy is being called for the element.
This is unexpected, since the usual behavior for Hyperapp, when it encounters a node with the same tagName as the previous render, it will not destroy and recreate the element, but rather reuse it. (unless the node is keyed differently -- but that's not the case here)
Oddly, this destruction and recreation of the element can be prevented by adding a static key to the p, but that's not something one normally would need, since the vdom structure is identical each render.
So is this a serious problem? Not really. But it's unexpected if you believe yourself to understand how keys work and when their needed and not. In particular it can be confusing when using the router together with @hyperapp/transitions, as in this issue: https://github.com/hyperapp/transitions/issues/10
@jorgebucaran Nope that's not it. The thing is that the root (of each Page component) is unchanging. It's the same no matter which page you are routed to. Even so, for some reason Hyperapp is destroying and recreating the element every time I navigate to a different page, (causing ondestroy to be called on it).
So to clarify: the unexpected thing, in the example above, is that the ondestroy handler is being called, each time I navigate between pages.
It's quite common for people to have the problem that lifecycle event handlers are not being called. But this is the opposite problem. I don't understand why ondestroy is being called.
@zaceno ~~Good catch, this is somehow related to lazy components.~~ See below for a more accurate diagnosis.
@zaceno I narrowed it down to resolveNode. The problem is resolveNode resolves to "" for null and undefined virtual nodes, which in the case of the Route component, is the case when a route doesn't match.
This means we end up comparing either the new node or oldNode (<p> in the example) against an empty string!
The correct solution is a proper fix to @jonlov's https://github.com/hyperapp/hyperapp/pull/636.
Aha! Makes perfect sense. Also explains how I was able to work around this behavior by using a common key for the root node of the two pages. Thanks!
I introduced lazy components because of the boilerplate required to access the state and actions. In 2.0, since actions are no longer "wired" to your app, parent components only need to pass state into children components. So, I see this feature better directed to enable dynamic views instead (which are actually lazy).
If you wrap the routes with the <Switch> component it doesn't destroy the parent components. I'm not sure if we should change resolveNode to resolve to an empty element or leave it as it is. 🤔
If we return null on a component it shoudn't create any element, but it should reuse the old element if it was the same. How can we do this?
Does this mean that resolving "" is wrong?
So, I see this feature better directed to enable dynamic views instead (which are actually lazy).
I mean that I'll reuse the implementation of lazy components to implement dynamically imported components import(), but 1.0 lazy components will be removed from 2.0.
But I don’t think that it is a bug that needs solving.
Just requiring that every element that has any lifecycle methods associated with it be keyed would solve that problem.
https://codepen.io/anon/pen/XqqrYV?editors=1111
