add modifiers to debug render tree

[chancancode]

I am not sure that it is a good idea to add more work here, especially to the close element operation which affects every single element we render which can add up. I don’t love the architecture of the inspector integration today, the code is not strippable, has some non-zero impact even when it’s not enabled, and IIRC it’s enabled pretty much any time that the inspector is installed, not just when the inspector is visible or the rendering tab is selected. We probably need to think harder about whether that is ok before adding even more work there, especially something as impactful as this.

Doing it for modifiers itself is probably not an issue, it’s the fact that you need some way to tie it to the element and that led you to add HTML nodes to the tree that is the problem. Beyond what I said above it will also make the tree vastly bigger and you may run into slowness in serializing the tree and sending it over to the inspector. That happens every time something changes and causes a re-render (even for no-op re-render).

If you just want to add modifier node without introducing html element parents here I think that would be fine. We still give you a reference to the element anyway and you can probably still group them in the inspector’s processing code if you want.

[patricklx]

@chancancode i tried to improve the performance by checking with a Set. It actually only creates the html element if there are modifiers. I also wanted components that are under the html element to actually appear under the element. If this is still not okay, i could also just create the html element in the pushModifiers call and close it immediately afterwards. That shouldn't cause much more perf issues

[patricklx]

@chancancode i update the PR. by the way, inspector does not serialize+send every time something changes. I did improve the inspector long time ago to limit the calls to debug render tree (and serialize+send) to once per 250ms. Maybe if someone has many items of components+modifiers it starts slowing down . But I also have an idea for that, at least at inspector side.

[chancancode]

Gonna circle back to the inspector stuff after finishing #1568 (which could use a review from you)

[patricklx]

I found that in-element needed to add a destructor to release it in the debug render tree. Does this also need something similar?

[patricklx]

I think i move everything to https://github.com/glimmerjs/glimmer-vm/blob/17ca94fdd58c5730c5f8b6f523e348ec03873f69/packages/%40glimmer/manager/lib/public/modifier.ts#L68