WebReflection / uland

A ยตhtml take at neverland
ISC License
108 stars 2 forks source link

[Help] Failed to execute 'setEndAfter' on 'Range': the given Node has no parent. #3

Closed djalmajr closed 4 years ago

djalmajr commented 4 years ago

Hello! I'm studying uland for a small project, but I'm facing some problems...

When I use html.for for a list of todos, the following error happens (only on Chrome... Firefox works as expected):

image

with this piece of code:

// src/containers/app.js
//...
<ul>
  ${filtered.map(todo => html.for(todo)`${Todo({ todo })}`)}
</ul>
//...

But, if I change to this:

// src/containers/app.js
//...
<ul>
  ${filtered.map(todo => Todo({ todo })}}
</ul>
//...

The error goes away, but the benefits of use keyed list is lost.

I'm rerendering the app in this way:

// src/index.js
//...
const wrapper = document.querySelector("#root");
const renderApp = () => render(wrapper, Component(App));

renderApp() && store.subscribe(renderApp);

P.S: checked attribute not work as expected in Chrome or Firefox (file src/containers/todo.js)

image

Link to the test project: https://codesandbox.io/embed/todo-uland-xy5rk?file=/src/index.js

WebReflection commented 4 years ago

Few things:

Thanks for collaborating and eventually helping me out to better help you out ๐Ÿ‘‹

WebReflection commented 4 years ago

P.S. conditional renders, like in react, might cause issues too ... always return the same node from each component, and use eventually aria-hidden=${true || false} instead of returning an empty html, as that messes up completely the keyed logic, and also the eventual nested components logic ...

// TL;DR this is wrong
if (condition)
  return html``;
// ... because the key is on the node
return html`<div>a real node</div>`;

The following is better, and it's also cheaper, as there are less mutations on the DOM

return html`<div aria-hidden="${hideCondition}">a real node</div>`;
// or ...
return html`<div .hidden="${hideCondition}">a real node</div>`;
// or
return html`<div class=${condition ? 'hidden' : ''}>a real node</div>`;
djalmajr commented 4 years ago

P.S. conditional renders, like in react, might cause issues too ... always return the same node from each component, and use eventually aria-hidden=${true || false} instead of returning an empty html, as that messes up completely the keyed logic, and also the eventual nested components logic ...

// TL;DR this is wrong
if (condition)
  return html``;
// ... because the key is on the node
return html`<div>a real node</div>`;

The following is better, and it's also cheaper, as there are less mutations on the DOM

return html`<div aria-hidden="${hideCondition}">a real node</div>`;
// or ...
return html`<div .hidden="${hideCondition}">a real node</div>`;
// or
return html`<div class=${condition ? 'hidden' : ''}>a real node</div>`;

Awesome!

djalmajr commented 4 years ago

Few things:

  • uhtml handles keys automatically, but uland always does the keying for you, there are really few reasons to use html.for, but if you have IDs, you better use it via html.for(sameReference, todo.id) or ... just don't bother with html.for, as it's not needed
  • checked is a boolean attribute, or a native accessor, with these kind of attributes, in uhtml, you want to use .checked=${boolean} instead of checked=${boolean} ... see About Attributes

I did these adjusts and the error goes away, however ๐Ÿ˜… the following occurs:

todo-keyed

The input content of active todo is filled with the content of completed todo... How I solve this without key?

  • the project is quite articulated for a todo, but mostly I have zero time these days to review big projects ... try to keep it simple, see if after avoiding html.for and using .checked it works as meant, try to see if using neverland instead works, as it's based in lighterhtml which does something slightly different, but on top of this, if you think there is a bug, try to isolate the bug and show me the failing code, as going through dozen files is not helping me much

I made this way to feel how would be in a more complex project :)

Thanks for collaborating and eventually helping me out to better help you out

Thank you for the quick response. I like the way you do some things and I would like to use uland (and ยต-related) instead neverland for his tiny size.

WebReflection commented 4 years ago

if the input uses a value, then go .value=${...} instead ... same as .checkbox, these behaviors are defined by the DOM standard, value as attribute, instead of accessor, won't change the visible input content

djalmajr commented 4 years ago

if the input uses a value, then go .value=${...} instead ... same as .checkbox, these behaviors are defined by the DOM standard, value as attribute, instead of accessor, won't change the visible input content

One single character did the thing ๐ŸŽ‰๐ŸŽ‰! Thanks again... I will try a more complex project.

WebReflection commented 4 years ago

@djalmajr if interested, I've explained in here why that's the case: https://github.com/WebReflection/uhtml/issues/21#issuecomment-658678403