[WebProfilerBundle][WIP] Real-time twig debug, aka xray view #28056

ro0NL · 2018-07-24T19:08:05Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#...
License	MIT
Doc PR	symfony/symfony-docs#...

This PR aims to enable real-time twig debugging, aka xray view, aka layout inspection, etc.

Inspired by https://github.com/beyondcode/laravel-view-xray me and @yceruto tried to port this into Symfony/Twig.

See https://github.com/yceruto/twigxray for the initial work done. This PR is mostly a direct port to move the discussion to Symfony core.

What this PR is about

Live demo

https://jsfiddle.net/r30g4boL/5/

However, there's a big technical limitation. We cant really tell if the output is truly intended HTML, and as such the hidden HTML comments we're adding could break things. And it does :) hence WIP.

This must be handled server-side, and to make it really robust i want to propose the following steps to finalize this;

Twig bundle configuration

twig:
    html_debug:
        enabled: false # for BC
        included: ['*.html.twig']
        excluded: []

The included filter reduces most of the unwanted templates, and can be further downsized using the excluded filter. E.g. webprofiler would prepend it's own templates here, to effectively disable HTML debugging globally.

Twig syntax

At least the following block will break forms:

symfony/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig

Lines 58 to 60 in 0eea077

    
           {% block form_group_class -%} 
        
           col-sm-10 
        
           {%- endblock form_group_class %}

But also something like <title>{% block title %}Welcome!{% endblock %}</title> leads to unwanted HTML comments.

To avoid this per case we need some modifier at the syntax level, so the developer can clarify the intend, think:

{% invisible %} ... {% endinvisble %}

Any block etc. within this boundary is free from HTML debug comments wrapped arround it.

Thoughts?

javiereguiluz · 2018-07-25T06:46:27Z

I warmly appreciate your work to prepare this proposal. However, before getting into technical details, could we please discuss about this proposal in general?

For example, my biggest concern is that I've never needed this feature myself and I can't see me using this in any of my Symfony apps. In other words: it solves a problem I've never had. But this is just my own experience ... so I'd like to read other people comments. Thanks

ro0NL · 2018-07-25T06:52:36Z

could we please discuss about this proposal in general?

Yes please :) that's why opened the PR as early as possible. I know you prefer RFCs/issue first, but i wanted to try this and show a real POC.

Also see twigphp/Twig-extensions#194 where ppl are upvoting this idea :)

Personally i'd be very happy with "Visual element > Click > Source file" as a workflow.

nicolas-grekas · 2018-07-25T07:20:46Z

I like that. Not sure about "invisible" yet: this asks authors write directives for a debugging tool.
About the implementation, would it be possible to get rid of the comments and replace them with a source map? I.e. keep track of the offsets in the output and map them to source in a separate artifact?

ro0NL · 2018-07-25T07:32:44Z

this asks authors write directives for a debugging tool

Correct, and it's not ideal. Then again if such a modifier lives in twig core, it would be a no-op in case debugging is disabled / not available. That allows to "fix" e.g. upstream templates independently. To me that's important to make it viable long-term.

About the implementation

The thing is.. we need the comment boundaries server-side, to actually get us the offsets client-side. But i like the way you're thinking :) Maybe we can inject a hidden unique UTF-8 char sequence to use as a boundary. Then it should be possible to dump a mapping of boundary indexes to source data.

Conceptually injecting a hidden UTF-8 can be just as wrong as injecting a HTML comment, but would be less likely to break i guess 🤔

nicolas-grekas · 2018-07-25T08:11:30Z

Could it work with random unique string used as a marker, that we would remove before returning the output and that would help build the source map?

ro0NL · 2018-07-25T08:37:37Z

Not really into twig internals, that's why i asked @yceruto :)) but i can imagine it would work like a shadow root. So one big comment at the bottom of the page containing the actual rendered templates which we can match with what's in the actual DOM. That gives us the regions client side, but increases the amount of rendering and im not sure how reliable the match would be.

magarzon · 2018-07-25T08:39:14Z

I think this is very useful, I find myself very often going to the profiler twig section to see which twig files are involved in some pages, specially if it's not my code (for example, I have been trying to adapt eZPlatform to my project, and it's a nightmare to guess which twig files is using)

But, as it is, I couldn't use it in that case, because it force me to change the external twigs from the third party to enable this feature (if I'm not wrong)

dkarlovi · 2018-07-25T08:40:34Z

First, let me say I like this a lot, having this info available would be invaluable to UI people who are doing HTML/CSS + Twig, but not PHP.

I'd include a screenshot from the fiddle in the PR description to explain what this is about at a glance, wasn't clear to me.

The color picker is not needed IMO, you can easily find a highly contrasting color with JS and use that, it can even be different as you move across the page.

ro0NL · 2018-07-25T08:54:59Z

@dkarlovi if we can automate the color approach that be great i guess. Like to see a fiddle for that :) as im not sure how it would work technically (im drawing a boundary box positioned above all elements between start/end comments). Either way, dealing with the color/contrast is a must.

because it force me to change the external twigs from the third party to enable this feature (if I'm not wrong)

@magarzon you're right :) but only if those templates are breaking. Then they need a upstream fix. Could be done with the suggested {% invisible %} syntax, or until then exclude the template in your app, override it, or fully disable debugging.

Ideally it works out-of-the-box, and we're trying to find a way how that could be done :)

dkarlovi · 2018-07-25T09:02:38Z

@ro0NL there's window.getComputedStyle() which would allow to fetch information about the element you're hovering and figure out the contrasting color by simple math. I don't have time for a fiddle right now, but in theory it sounds straightforward.

Update: simple math.

ro0NL · 2018-07-25T09:09:50Z

about the element you're hovering

@dkarlovi but that's the issue, we don't hover an individual element in source. We hover a computed region element, which visually spans multiple elements in source.

Currently the background color (default red) is controllable, and the text color (region tooltip) is inverted using CSS.

Also not sure which computed color we'd use (border, background, text?)

linaori · 2018-07-25T09:59:06Z

I've needed this feature before, especially with includes/render-controller places or multiple includes it would make it a lot easier to find out which part of the template belongs to what.

yceruto · 2018-07-25T13:15:46Z

... would it be possible to get rid of the comments and replace them with a source map? I.e. keep track of the offsets in the output and map them to source in a separate artifact?

Could it work with random unique string used as a marker, that we would remove before returning the output and that would help build the source map?

This makes me think and I want to believe that's possible as long as we keep the advantage of inspecting dynamic content (i.e. one template that is loaded through AJAX)

stof · 2018-07-25T15:57:50Z

As @fabpot already rejected adding such feature in Twig core, I doubt he will agree adding it in Symfony core. Could it be implemented as a thrid-party bundle instead ?

ro0NL · 2018-07-25T17:59:30Z

Sure, but conceptually it needs to solve the same problem. I think from a bundle pov it would come with it's own tag to let the user define debug-able regions. As such it wouldnt be compatible with any vendor template out there today. I was striving for the opposite, at least a noble goal on itself :))

As @fabpot already rejected adding such feature

The ref i found dates back to 2016 and is still open. I agree if we can't nail it, we shouldnt do it. And that's a big if at the moment. The feature on itself is priceless.

On the technical side; another idea was to parse the result HTML; check for all semantically valid debug comments; compute the invalid ones and remove those.

stof · 2018-07-27T11:42:27Z

@ro0NL the existing PR on Twig-Extensions relies on a configurable blacklist of templates instead of forcing templates to add a tag when they don't want to be wrapped (which would also create an extra dependency as long ad the tag is not part of Twig itself).

Here, you added a hardcoded blacklist, and then you talk about adding a tag (which might not even work to solve this need)

stof · 2018-07-27T11:48:14Z

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/twig.html.twig

+            <input type="color" id="__twig-debug-color" value="#ff0000">
+        </div>
+        <div class="sf-toolbar-info-piece">
+            <span><a href="javascript:;" id="__twig-debug-enable">Enable debug</a></span>&nbsp;


href="javascript:;" breaks the CSP compatibility. Use button tags instead (which are semantically correct) or href="#" with event.preventDefault() in the listener (more complex due to browser compat as this is part of the toolbar, not of the profiler UI for which we can afford supporting only modern browsers).

stof · 2018-07-27T11:48:17Z

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/twig.html.twig

+        </div>
+        <div class="sf-toolbar-info-piece">
+            <span><a href="javascript:;" id="__twig-debug-enable">Enable debug</a></span>&nbsp;
+            <span><a href="javascript:;" id="__twig-debug-discover" style="display: none;">Discover</a></span>


inline styles should be avoided for CSP compat. Use a class with styles instead.

stof · 2018-07-27T11:52:16Z

src/Symfony/Bundle/WebProfilerBundle/Twig/Node/HtmlDebugEnterComment.php

+/**
+ * @author Yonel Ceruto <yonelceruto@gmail.com>
+ */
+class HtmlDebugEnterComment extends Node implements \Twig_NodeOutputInterface


Symfony uses the namespaced version of all Twig classes and interfaces, to make sure we are ready for the future version of Twig dropping support for the non-namespaced version.

Yeah, should Twig\Node\NodeOutputInterface be used instead.

stof · 2018-07-27T11:54:26Z

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig

+                        bar.textContent = label;
+                    }
+                    bar.style.top = parseInt(rect.top + (document.documentElement.scrollTop || document.body.scrollTop) + rect.height) + 'px';
+                    bar.style.left = parseInt(rect.left + (document.documentElement.scrollLeft || document.body.scrollLeft)) + 'px';


parseInt is wrong here, as you don't give it a string as input, but a number.

stof · 2018-07-27T11:57:20Z

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/base_js.html.twig

+            return !!document.getElementById('__twig-debug-bar');
+        };
+
        var twigDebugEnable = function () {


the logic in this method is not compatible with old browsers, which is not good for something running in in the toolbar (as the toolbar runs in the userland page, which might need to support them).

so there is 2 solutions here:

ensure the code is compatible with old IE versions

feature-detect the support for features being used, and forbid enabling the Twig debug mode if not supported.

ro0NL · 2018-07-27T13:41:18Z

@stof the issue is e.g. https://github.com/symfony/recipes/blob/master/symfony/twig-bundle/3.3/templates/base.html.twig

we dont want to exclude the entire template, assuming it has more valueable blocks/regions in real life projects. However, within this template we do need to exclude

<title>{% block title %}Welcome!{% endblock %}</title>

That would be solved with a tag / at the syntax-level. But i prefer to autodetect this as suggested previously by parsing the actual HTML and remove invalid debug comments.

artursvonda · 2018-08-03T10:52:55Z

If the issue is to understand which templates are used in any given place, an alternative approach would be to render separate ("source") view where we can highlight ANY part of ANY format and tell user where it came from. This would render actual source (text) and thus doesn't care about format of the content (could do this for JSON too). Downside is that it's not "inline" and doesn't work like "Inspect Element" in browsers.

fabpot · 2020-02-03T11:08:25Z

I propose to close this PR as it is stale. I'm still not convinced that it should be in core. Would be happy to see a bundle doing that though.

[WebProfilerBundle] Real-time twig debug, aka xray view

b3f2e26

carsonbot added Status: Needs Review WebProfilerBundle Feature labels Jul 24, 2018

handle scroll/resize

a09ecd4

ro0NL mentioned this pull request Jul 25, 2018

DisplayCallGraphTemplate: a visual way to show the display graph twigphp/Twig-extensions#194

Closed

nicolas-grekas added this to the next milestone Jul 25, 2018

stof requested changes Jul 27, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jul 27, 2018

ro0NL mentioned this pull request Dec 8, 2018

[HackDay][WebProfilerBundle]Display the main template on the debug toolbar #29534

Closed

ro0NL mentioned this pull request Jun 25, 2019

[Profiler] HTML comments for twig blocks #32175

Closed

fabpot closed this Feb 3, 2020

ro0NL deleted the twig-debug branch February 3, 2020 11:16

nicolas-grekas modified the milestones: next, 5.1 May 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[WebProfilerBundle][WIP] Real-time twig debug, aka xray view #28056

[WebProfilerBundle][WIP] Real-time twig debug, aka xray view #28056

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	{% block form_group_class -%}
	col-sm-10
	{%- endblock form_group_class %}

Uh oh!

[WebProfilerBundle][WIP] Real-time twig debug, aka xray view #28056

[WebProfilerBundle][WIP] Real-time twig debug, aka xray view #28056

Uh oh!

Conversation

Uh oh!

What this PR is about

Live demo

Twig bundle configuration

Twig syntax

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!