mbasso / asm-dom

A minimal WebAssembly virtual DOM to build C++ SPA (Single page applications)
https://mbasso.github.io/asm-dom
Other
2.79k stars 88 forks source link

Regression: children aren't displayed when appended manually to a parent with no child. #24

Closed ArthurSonzogni closed 5 years ago

ArthurSonzogni commented 5 years ago

I discovered my example: https://github.com/ArthurSonzogni/asm-dom-starter/blob/master/src/index.cpp

doesn't work anymore on version 0.6.2

Here is the relevant snippet:

  // Limit the number of actions to 10.
  asmdom::VNode *action_list = <ul class = "action_list"></ ul>;
  int start = std::max(0, (int)actions.size() - 10);
  for (int i = start; i < actions.size(); ++i) {
    std::string label = "action[" + std::to_string(i) + "] = " + actions[i];
    action_list->children.push_back(<li>{label}</ li>);
  }

I tried adding one child in the initial action_list element. Then my manually added children are properly displayed.

It looks like a regression. Or maybe I was using it the wrong way.

mbasso commented 5 years ago

Hi @ArthurSonzogni, in the latest version of asm-dom a normalization of VNodes is executed immediately after their creation. So, in this case new children are simply ignored... It will continue to work after the merge and release of PR #21, since the normalization will take place only when really needed (when patch is called). However, adding children after the creation of a VNode is not recommended, it should be created using CPX or the h function and then never modified by the user. With CPX you can set children dynamically in the way defined here, using asmdom::Children.

I think we can leave this issue open and update the doc to highlight it. What do you think? Let me know if it works fine after that little change 😄

ArthurSonzogni commented 5 years ago

Yes, I am now using it with:

asmdom::Children children
for(auto it = ..)
  children.push_back(it)

return <div> {...children} </div>

Which looks better to me.

Feel free to close this bug. I just observed something regressed, but not supporting this looks good to me.

mbasso commented 5 years ago

Perfect! I'll close this