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.
| I'm finishing this pr in #1212. Thanks for your contribution! |
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.