solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.12k stars 915 forks source link

Pass previous node in universal insertNode #1148

Open ranfdev opened 2 years ago

ranfdev commented 2 years ago

Describe the bug

I've just implemented a custom renderer to GTK4, using solid-js/universal. It works pretty well, even though it requires some little workarounds.

Gtk.Box, which is one of the most used GTK widgets, has a method insert_child_after(child, previous_sibling), while solid's universal insertNode only provides the sibling after: From the solid universal renderer example:

insertNode(parent, node, anchor) {
    parent.insertBefore(node, anchor);
},

what GTK requires me to do:

insertNode(parent, node, anchor) {
    parent.insert_child_after(node, ???????);
},

Steps to Reproduce the Bug or Issue

Implement a custom native renderer using gtk

Expected behavior

I think solid may already have the information about the previous_sibling internally. If that's the case, it would be nice exposing that sibling in insertNode so that people can write

insertNode(parent, node, next_sibling, prev_sibling) {
    parent.insert_child_after(node, prev_sibling);
}

I think I'm able to workaround this issue for many GTK widgets, but having the prev_sibling directly available would be nicer

ryansolid commented 2 years ago

Ok interesting. I just copied the DOM APIs which meant we used same reconciler and same methods. It may be possible as you say. In the DOM if you don't have an anchor, like it is null, it inserts it at the end. And the parent is aware of their children so it can do that.

chiefcll commented 1 month ago

@ryansolid - Another caveat of this that I am just running into is reconcilingArray. I have an array of items rendering via and if I change the order of the items in the array, the reconciler uses

image

The newNode being inserted already exists as a childNode... My understanding with DOM using insertBefore the DOM will MOVE the element. I wasn't doing this as I expected the newNode to be removed and then inserted. Instead I was creating two entries of the newNode in the children list. Depending on how the order is changed (moving up or down in the list), sometimes it replaces the node.

I can make the change on my end but feel like this is an important difference for Universal Rendering and how the DOM does rendering. I was expecting the node to be removed and then inserted, but that is not the case.

ryansolid commented 1 month ago

@chiefcll this is a really good point and I don't know how other universal renderers handle it to be fair. I have only ever made DOM based custom renderers so identifying a move is not something I have done.

I guess the safest approach is to have an internal insertNode.

function insertNodeInternal(parent, newNode, oldNode) {
  const newParent = getParentNode(newNode);
  if (newParent) removeNode(newParent, newNode);
  insertNode(parent, newNode, oldNode);
}

I guess I'm wondering if there are other platforms where nodes can be attached in multiple places etc.. This is pretty wide open when we assume things don't work DOM like.