Change inline node deprecation and fix deprecations in the codebase #1184
Conversation
The `string $content` parameter is a legacy from before Markdown. It is redundant and replaced by `array $children`.
wouterj
force-pushed
the
inline-content-deprecation
branch
from
37f375b to
2397209
Compare
wouterj
force-pushed
the
inline-content-deprecation
branch
from
2397209 to
49312b9
Compare
wouterj
force-pushed
the
inline-content-deprecation
branch
from
49312b9 to
10f7ca1
Compare
wouterj
force-pushed
the
inline-content-deprecation
branch
from
10f7ca1 to
a1cc381
Compare
We cannot fix these PHP deprecations, as these are happening in third party code (and fixing them requires a major version bump of Flysystem). PHPUnit 11 distinguishes direct and indirect deprecations, allowing us to automatically ignore all indirect deprecations. However, this version requires PHP 8.2, while this library still supports PHP 8.1.
wouterj
force-pushed
the
inline-content-deprecation
branch
from
a1cc381 to
832f346
Compare
| @@ -1 +1 @@ | |||
| *{{- node.value|raw -}}* | |||
| *{{- node|plaintext -}}* | |||
There was a problem hiding this comment.
When I introduced the inline nested nodes I did use the value because that would not require a change in the templates. As templates can be overwritten by consuming projects. I did see that as a backward compatibility issue.
This was the reason to try to be backwards compatible and accept the issues in our pipeline. How do you see that? Bc in templates is a very complex problem as I experienced in the past. But maybe I will try to be too strict about this project?
There was a problem hiding this comment.
I'm not sure I can follow you. The original content still works, but triggers a deprecation. Are you thinking of a case where the user overrides getValue() of a node that now has a BC break because it isn't called anymore?
You can use Deprecation::triggerIfCalledFromOutside() in getValue() then (or the Symfony tactic: add a virtual new boolean parameter to getValue() that determines whether the deprecation should be triggered). This should not produce a deprecation when called from a Guides template, but does when called by a user.
In general, deprecations are most useful if all of them can be fixed by a user without upgrading the package triggering the deprecation. If Guides triggers a deprecation from itself, I would never know if I can safely upgrade to 2.0.
After upgrading to the latest guide for the first time since a long time, I saw quite some deprecations of #1162 coming from Guides itself. This PR enables deprecations in the testsuite, so that any deprecated usage in the library is noticed and fixed.
While working on that, I discovered we can take #1162 one step further. That PR adds a
array $childrenparameter to some inline nodes that must be used instead of the existing$valueparameter, but it doesn't actually deprecate the$valueparameter.This PR:
$valueparameter with the new$childrenparameter (in a BC matter)$childrenparameter locationAt last, enabling deprecations showed a couple other deprecations that I've fixed as well to make sure the tests are green.
Sorry for the slightly chaotic PR contents. I hope it all makes sense now and happy to elaborate further if needed.