fix: text nodes should be taken into account by _render #129

felippe-regazio · 2023-01-21T03:02:06Z

_render is not taken text nodes into account. this leads to situations like this: imagine the following component

    return (
      <>
        <h1>{this.data.count}</h1>

        <button onclick={() => this.data.count--}>
          Dec
        </button>
        
        <button onclick={() => this.data.count++}>
          Inc
        </button>

        { this.props.children }      
      </>
    )

note that the { children } are the last node collection. now imagine that we called this component like this:

  <MyComponent>
      asdf
  </MyComponent>

we will get this result:

<my-custom-element>
    asdf
  <h1>0</h1>
  <button>Dec</button>
  <button>Inc</button>
</my-custom-element>

im having this problem using the "web component mode" because of this part of the code:

        /** customElementsMode.ts - Line 43 **/
        const children = Array.from(this.children).map(c => render(c))

        // because nano-jsx update need parentElement, so DocumentFragment is not usable...
        const el = h(
          'div',
          null,
          _render({
            component,
            props: {
              children: children,
              ref: (r: any) => (ref = r)
            }
          })
        )

the _render will be called and wont see the text node which will be ignored by the handles.
obs: im not sure if its possible to use Node.TEXT_NODE here taking different envs into account, but would be the safer way.

yandeu · 2023-01-21T12:17:52Z

I tried to write a browser test for Web Components (felippe-regazio@39f25bf), but since I don't exactly know how it supposed to work, I don't know how to write the test. Maybe you can test the case you mentioned above?

The customElementMode was written by @Shinyaigeek, maybe he can help with some details? And also with #130?

felippe-regazio · 2023-01-22T03:05:52Z

Some help from @Shinyaigeek would be great, for sure. I will wait for his opinion (in case of he asks to change something or have a better ideia about the behavior), an then I will proceed with the tests.

Shinyaigeek · 2023-01-27T14:09:56Z

I looked at the 39f25bf commit. I feel it is better to register a my-custom-element component by running defineAsCustomElements in the <script /> in the <head />, and write out the my-custom-element component in the body tag as HTML, and then write the test code in the <script /> at the bottom of the <body /> in order to write tests about web-component behavior considering the usual way to use web-components

like:

<html>
  <head>
  ...
  <script src="nanojsx" />
  <script type="module">
  // defineAsCustomElements
  </script>
  </head>
  <body>
  <my-custom-element />
  <script type="module">
  // test code
  </script>
  </body>
</html>

yandeu · 2023-01-28T21:04:53Z

to write tests about web-component behavior considering the usual way to use web-components

Good idea. Thanks!

felippe-regazio and others added 3 commits January 20, 2023 22:08

fix: text nodes should be taken into account by _render

6b78b44

Replace Node.TEXT_NODE with corresponding enum val

4950aa5

Test webComponent

39f25bf

Shinyaigeek mentioned this pull request Jan 27, 2023

When in "Custom Elements Mode", Shadow Dom should be configurable and optional #130

Merged

Improve web component test

4737784

yandeu merged commit 04e54cd into nanojsx:master Jan 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: text nodes should be taken into account by _render #129

fix: text nodes should be taken into account by _render #129

felippe-regazio commented Jan 21, 2023

yandeu commented Jan 21, 2023

felippe-regazio commented Jan 22, 2023

Shinyaigeek commented Jan 27, 2023 •

edited

Loading

yandeu commented Jan 28, 2023

fix: text nodes should be taken into account by _render #129

fix: text nodes should be taken into account by _render #129

Conversation

felippe-regazio commented Jan 21, 2023

yandeu commented Jan 21, 2023

felippe-regazio commented Jan 22, 2023

Shinyaigeek commented Jan 27, 2023 • edited Loading

yandeu commented Jan 28, 2023

Shinyaigeek commented Jan 27, 2023 •

edited

Loading