Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add current element to in-element #1575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patricklx
Copy link
Contributor

No description provided.

@chancancode
Copy link
Contributor

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.

@patricklx
Copy link
Contributor Author

patricklx commented Mar 9, 2024

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
Copy link
Contributor Author

patricklx commented Mar 9, 2024

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

@NullVoxPopuli
Copy link
Contributor

dang, that looks real nice @patricklx 🎉

@patricklx
Copy link
Contributor Author

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 patricklx force-pushed the add-previous-element-to-in-element branch from eb09c8b to e6ba8a7 Compare April 4, 2024 08:24
@patricklx
Copy link
Contributor Author

@chancancode can you have another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants