Skip to content

Commit

Permalink
bug #23050 [Form][Profiler] Fixes form collector triggering deprecati…
Browse files Browse the repository at this point in the history
…ons (ogizanagi)

This PR was merged into the 3.3 branch.

Discussion
----------

[Form][Profiler] Fixes form collector triggering deprecations

| Q             | A
| ------------- | ---
| Branch?       | 3.3 <!-- see comment below -->
| Bug fix?      | yes
| New feature?  | no <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

Since 3.3, if you inspect your logs when accessing the form profiler panel, you'll see some of these:

```sh
php.INFO: User Deprecated: The Symfony\Bundle\WebProfilerBundle\Twig\WebProfilerExtension::dumpValue() method is deprecated since version 3.2 and will be removed in 4.0.
[...] at /src/Symfony/Bundle/WebProfilerBundle/Twig/WebProfilerExtension.php:119
```

The [WebProfilerExtension](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/WebProfilerBundle/Twig/WebProfilerExtension.php#L73) is still using a `ValueExporter` instance for BC reasons when the $value ins't an instance of `Data` and this BC layer will be removed in 4.0 (so it'll throw an exception/error when trying to use it with something else than a `Data` instance).

The issue is since #21638, collectors (including forms one) have been drastically simplified to leverage the "seamless usage of Data clones", which is great!... But there is a slightly different implementation between `Data::seek()` and [`Data::__get()`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/VarDumper/Cloner/Data.php#L123-L130). There are probably good reasons for this, but it prevents from using classic Twig property access when the underlying data may be a scalar (`null`, `false`, ...).

I already spot that while working on the [Validator panel](https://github.com/symfony/symfony/pull/22554/files#diff-deac3c5ce4aa87243093dcd6a3f77a56R84). Perhaps there is a better solution, though.

Anyway, current `master` is currently broken, as it still tries to use the `ValueExporter`, which is already removed. And removing the BC layer in `WebProfilerExtension` isn't enough for now. It needs this fix.

BTW it also fixes rendering of the concerned inlined-dumps:

|Before|After|
|--|--|
|<img width="818" alt="screenshot 2017-06-03 a 13 35 25" src="https://cloud.githubusercontent.com/assets/2211145/26753222/01a692e6-4862-11e7-90d5-9cc9e4832648.PNG">|<img width="817" alt="screenshot 2017-06-03 a 13 35 47" src="https://cloud.githubusercontent.com/assets/2211145/26753224/090d5d6c-4862-11e7-87c1-73d5346f602c.PNG">|

Commits
-------

9de898d [Form][Profiler] Fixes form collector triggering deprecations
  • Loading branch information
fabpot committed Jun 3, 2017
2 parents 6b9ff81 + 9de898d commit 5859703
Showing 1 changed file with 7 additions and 7 deletions.
Expand Up @@ -525,21 +525,21 @@
<th class="font-normal" scope="row">Model Format</th>
<td>
{% if data.default_data.model is defined %}
{{ profiler_dump(data.default_data.model) }}
{{ profiler_dump(data.default_data.seek('model')) }}
{% else %}
<em class="font-normal text-muted">same as normalized format</em>
{% endif %}
</td>
</tr>
<tr>
<th class="font-normal" scope="row">Normalized Format</th>
<td>{{ profiler_dump(data.default_data.norm) }}</td>
<td>{{ profiler_dump(data.default_data.seek('norm')) }}</td>
</tr>
<tr>
<th class="font-normal" scope="row">View Format</th>
<td>
{% if data.default_data.view is defined %}
{{ profiler_dump(data.default_data.view) }}
{{ profiler_dump(data.default_data.seek('view')) }}
{% else %}
<em class="font-normal text-muted">same as normalized format</em>
{% endif %}
Expand Down Expand Up @@ -571,21 +571,21 @@
<th class="font-normal" scope="row">View Format</th>
<td>
{% if data.submitted_data.view is defined %}
{{ profiler_dump(data.submitted_data.view) }}
{{ profiler_dump(data.submitted_data.seek('view')) }}
{% else %}
<em class="font-normal text-muted">same as normalized format</em>
{% endif %}
</td>
</tr>
<tr>
<th class="font-normal" scope="row">Normalized Format</th>
<td>{{ profiler_dump(data.submitted_data.norm) }}</td>
<td>{{ profiler_dump(data.submitted_data.seek('norm')) }}</td>
</tr>
<tr>
<th class="font-normal" scope="row">Model Format</th>
<td>
{% if data.submitted_data.model is defined %}
{{ profiler_dump(data.submitted_data.model) }}
{{ profiler_dump(data.submitted_data.seek('model')) }}
{% else %}
<em class="font-normal text-muted">same as normalized format</em>
{% endif %}
Expand Down Expand Up @@ -630,7 +630,7 @@
{% if resolved_option_value == option_value %}
<em class="font-normal text-muted">same as passed value</em>
{% else %}
{{ profiler_dump(data.resolved_options[option]) }}
{{ profiler_dump(data.resolved_options.seek(option)) }}
{% endif %}
</td>
</tr>
Expand Down

0 comments on commit 5859703

Please sign in to comment.