Only require Twig when profiler is enabled

[ruudk]

Q	A
Bug fix?	no
New feature?	no
BC breaks?	yes
Deprecations?	no
Related tickets	fixes #300
Documentation	if this is a new feature, link to pull request in https://github.com/php-http/documentation that adds relevant documentation
License	MIT

What's in this PR?

Require Twig to be installed when the profiler is enabled. Move twig/twig dependency to suggest and require-dev

Why?

We don't have Twig installed in our project. HTTPlug requires it to be installed.

Checklist

• Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
• Documentation pull request created (if not simply a bugfix)

[ruudk]

I cannot test this (yet) because of #301

[xabbuh]

As written in #300 I do not think we should forbid using the profiler tools when Twig is not installed. Using them in tests does not require Twig if I do not miss anything.

[ruudk]

@xabbuh It's not only the Twig extension that requires Twig, right? Also by registering the profiler button in the web profiler it requires Twig?

Shouldn't we remove these services when Twig is not installed?

<service id="httplug.collector.collector" class="Http\HttplugBundle\Collector\Collector" public="false">
            <tag name="data_collector" template="@Httplug/webprofiler.html.twig" priority="200" id="httplug"/>
        </service>

        <service id="httplug.plugin.stack" class="Http\HttplugBundle\Collector\StackPlugin" public="false" abstract="true">
            <argument type="service" id="httplug.collector.collector"/>
            <argument type="service" id="httplug.collector.formatter"/>
        </service>

[ruudk]

I changed the PR. It now removes the Twig extension when Twig is not installed.

I think it's impossible to use the Symfony Web Toolbar without Twig, right?

[xabbuh]

The template attribute of the data_collector tag is used when the data_collector.templates parameter is populated. But this parameter is only evaluated inside the ProfilerController which the web debug toolbar and the profiler panels. This controller is part of the WebProfilerBundle which already has a dependency on Twig.

[ruudk]

@xabbuh Thanks for the explanation. I get it know.

So this PR should be good then?

[dbu]

thanks, this change makes sense to me. i think we should only remove the service if we loaded it. not sure if symfony would complain when trying to remove a definition that does not exist - but even if not, i want to avoid confusion

[dbu]

can you please include the change to composer.json in this PR as well, moving twig/twig to require-dev?

and can you please add a changelog entry? please use "1.15.0 [unreleased]" as heading.

[ruudk]

@dbu I applied the suggested changes.

[dbu]

thanks, looks good to me!