glimmer-vm icon indicating copy to clipboard operation
glimmer-vm copied to clipboard

add current element to in-element

Open patricklx opened this issue 1 year ago • 7 comments

patricklx avatar Mar 09 '24 07:03 patricklx

Sorry we didn't get to address this before the other PR was merged.

Ultimately I am not sure what you're looking to accomplish is inline with the type of information the DRT is aiming to provide. It just provides the logical hierarchy the successor to/replacement for the "view tree/parent-child views chain" in classic components. In this view the {{#in-element}} doesn't belong to any particular "element parent" per-se, other than the next closest component/route template. And there are also plenty of other "holes" (things not represented) in the tree and personally I don't think we'll aim to have, for example, every {{ }} and {{# }}...{{/ }} represented that way.

The type of views that you are looking to assemble seems like a better fit for some kind of source maps/source location linkage where you can tie things back closer to the original template source as-written. Even if you have the parent element, you still won't have the relative ordering within the parent, that you happen to be able to infer in the other cases (you'll basically need a collapsed bounds of some sort – which is not information we have):

<div>
  <First />
  <Second />
  {{#in-element}}...{{/in-element}} <-- you can't place this here
  <Third />
</div>

My preference would be to dissuade you from trying to group the tree that way (other than for modifiers, which I get that you have to attach it to something to make sense).

However, if I cannot convince you of that, I would much prefer adding a metadata bag than hacking/hijacking the instance slot. The instance is really meant for the object to show the end-user whenever you click on a row/send to $E, etc. The inspector really shouldn't have to do a lot of special casing/pre-processing of the DRT and more or less should be able to just present the information contained therein as-is. (On a related note, I was not trilled about having to expose the customer modifier manager's internal state bucket as the instance, but we are missing a getDeubgInstance or similar hook in the manager API to do anything other than that.)

Also just know that we may not always be able to provide this information, especially if, in the future, the code/timing moved around such that, e.g. the modifiers are constructed completely asynchronously and we don't have it the parent element handy in that spot in the code, we are not going to go out of the way to thread that information if it's expensive/hugely inconvenient in the new code.

chancancode avatar Mar 09 '24 13:03 chancancode

alright. on this:

<div>
  <First />
  <Second />
  {{#in-element}}...{{/in-element}} <-- you can't place this here
  <Third />
</div>

Why not? The ordering is already like that in the debug render tree. So i only move them a level down in the same order they already are. Note that i would only do this if there are modifiers. I create an html node manually, and then i put the modifiers under the html element and also components and in-element that are under that element. I can now which components are under the element by looking at it's bounds, but that currently doesn't work for in-element. On the same order as they were.

how can i add metadata? I only see through debugRenderTree.getNode().meta = x;

patricklx avatar Mar 09 '24 14:03 patricklx

this is how it would look like: image

It doesn't show this exact case. But maybe the idea i want to show is clearer? Elements, modifiers, in-element and components will appear under the html element node, iff one was added for modifiers

patricklx avatar Mar 09 '24 14:03 patricklx

dang, that looks real nice @patricklx 🎉

NullVoxPopuli avatar Mar 09 '24 14:03 NullVoxPopuli

On a side note: i actually would like to have all blocks rendered:) like, each, let, if. I also experimented with rendering helpers, but those need some other rendering concept to make sense

patricklx avatar Mar 09 '24 15:03 patricklx

@chancancode can you have another look?

patricklx avatar Apr 04 '24 12:04 patricklx

@chancancode

patricklx avatar Jun 12 '24 07:06 patricklx