Nested hooked components issue

chip4 commented 3 years ago

This is a bug for the issue discussed here: https://github.com/WebReflection/discussions/discussions/50

In summary the issue is that nested components wrapped with Component() sometimes don't display when they should.

Here's some html showing the issue:

<html>
  <body>
    <span>Loading...</span>
    <script type="module">
      import {
        render,
        html,
        Component,
        useState,
      } from "https://cdn.skypack.dev/uland";

      const Section = ({ title, children }) => {
        return html`
          <div style="border: solid;margin: 1em;">
            <h2>${title}</h2>
            ${children}
          </div>
        `;
      };

      const InnerSection = Component(({ title, children }) => {
        return html`
          <div style="border: solid;margin: 1em;">
            <h2>${title}</h2>
            ${children}
          </div>
        `;
      });

      const arr = ["One", "Two"];

      const createArr = () => Array.from(Array(2), () => Math.random() * 10);

      render(
        document.body,
        html`
          <div>
            ${arr.map((title) =>
              Section({
                title,
                children: html`
                  <div>
                    Contents!!!!!!
                    ${createArr().map((title) =>
                      InnerSection({ title, children: html`Inner Contents` })
                    )}
                  </div>
                `,
              })
            )}
          </div>
        `
      );
    </script>
  </body>
</html>

There should be 4 InnerSections in total on the page, but only the 2 under the 2nd Section show.

A workaround was provided in the linked discussion to use html.for(document.body, title) on the children field when calling Section.

I did a git bisect (testing against ./esm/index.js) and 93793bef87e5be89603498b511965db00fc17b81 came up as the commit that introduced this.

Also if I changed https://github.com/WebReflection/uland/blob/master/esm/index.js#L18:

const info = cache.get(template) || cache.set(template, createCache());

to

const info = cache.set(template, createCache());

to bypass the cache the issue went away, but that's more of a clue than a fix.

WebReflection commented 3 years ago

it looks like I've created a regression there, but at least now I have a test-case to cover. Thank you very much, and apologies for the late reply, I had busy days. I'll look into it, and probably I will rollback if I can't find a good solution based on changes landed in 0.11.

WebReflection commented 2 years ago

@chip4 sorry but I never got a chance to properly investigate the issue, but I could reproduce it was broken, so I've reverted the latest change, and published it again. I hope that's fine.

chip4 commented 2 years ago

Reverting the commit works just fine for me. Thanks for taking the time to address the issue even if you couldn't dig in and figure out exactly what the problem was in that commit. Much appreciated.

WebReflection / uland

Nested hooked components issue #16