Bad support on conditional rendering when return nulls
Closed this issue · 4 comments
I wrote the following code
const Hello = () => {
const flag = van.state(false);
return [
button({ onclick: e => flag.val = !flag.val }, "Click me"),
() => flag.val ? null : p("114514")
]
}
- Click button:
p('114514')
disappears as expected - Click button:
p('114514')
should come back but it doesn't
https://jsfiddle.net/uch6d7es/1/
The code simply suppose the return value of derive
is a single Node
(including primitive values that can be converted into Text
) and perform Node.replaceWith(Node)
once the state changes. However if we return a null
, we will not get a Node for replacing in next state changes, and thus failed to do anything further.
Simply we can create a Comment Node if get null
in conditional rendering to ensure we have a Node to replace in next state change, but I hope we can come up with better solutions for support returning arrays (acts like Fragment) as well.
For State-derived child nodes, if the binding function returns null or undefined, the DOM node will removed. Removed DOM node will never be brought back, even when the binding function would return a non-null/undefined value based on future values of the dependencies.
Proposed solutions:
const {button, p, div} = van.tags
const Hello1 = () => {
const flag = van.state(false);
return [
button({ onclick: e => flag.val = !flag.val }, "Click me"),
() => flag.val ? '' : p("1919810")
]
}
const Hello2 = () => {
const flag = van.state(false);
return () => div(
button({ onclick: e => flag.val = !flag.val }, "Click me"),
flag.val ? null : p("1919810")
)
}
van.add(document.body, Hello1(), Hello2())
Better Hello1
I see. This feature just sounds weird, seems like the code behaviors like that, and we explain it as a feature! (like BUD in minecraft if you know)
Hello1
,Hello2
This seems better to match the design of State-derived child nodes in vanjs. However, I think user will be much happy if we can just support these features but not avoid them.
Anyway, I'll make some changes to adopt my own ideas. This project is amazing, it would be better if not keen on reducing code size.
Hi @swwind,
Thanks for your feedback!
Using null
or undefined
to instruct the permanent removal of a DOM node is an intentional design choice. Alternatively, we could introduce a special remove
symbol to instruct the removal. We chose not to introduce an additional symbol since it's not necessary (Occam's razor). Additionally, if an empty string can be used to temporarily remove a DOM node (which can be brought back some time later), it will be redundant for null
or undefined
to mean the same thing.
Hope it explains well :-)