Change the deprecation message for null attribute values#3307
Change the deprecation message for null attribute values#3307mpdude wants to merge 8 commits intosymfony:2.xfrom
null attribute values#3307Conversation
null attribute valuesnull attribute values
| if (null === $value) { | ||
| trigger_deprecation('symfony/ux-twig-component', '2.8.0', 'Passing "null" as an attribute value is deprecated and will throw an exception in 3.0.'); | ||
| trigger_deprecation('symfony/ux-twig-component', '2.32.0', 'Passing null as value for the %s attribute will omit the attribute in 3.0. To keep the current result, pass true instead.', $key); | ||
| $value = true; |
There was a problem hiding this comment.
Not sure we should precise the key... I mean maybe we should not suggest passing null is deprecated for that attribute only.
"To keep the current result, pass true instead."
I think with the various changes we plan, we should avoid using "current" / "previous" / "next" or maintaining (or reading) the docs will become quickly really hard.
What about something like "Use true to add the attribute, null or false to remove it"
There was a problem hiding this comment.
I will try to reword that.
My initial idea was that it might make it easier for users to understand which attribute to search for. But I also realized that people might be collecting and grouping deprecation messages by type, and so having a different message for each attribute might not be helpful to them either.
Also, for example the XmlFileReader deprecated in Symfony 7.4 only says XML is deprecated but does not give the file name. So, omitting the attribute name will be in line with that.
There was a problem hiding this comment.
Now it's
Passing null as value to set an attribute is deprecated, pass true instead. In 3.0, the null value will remove the attribute.
Co-authored-by: Simon André <[email protected]>
Kocal
left a comment
There was a problem hiding this comment.
Thanks you for working on this!
aria-*attributes with afalsevalue will trigger a deprecation notice, saying that it will be"false"in 4.0 (...)
Couldn't it be done in 3.x?
Co-authored-by: Hugo Alliaume <[email protected]>
I don't think so, but I haven't tried it, only looking at the code. In 2.x, So if users would get a deprecation notice in 2.x, telling them that Thus I think we need the change here to give ourselves a little more legroom in 3.0 and something users could switch their Am I missing something? |
|
Not sure about test failures, seem unrelated |
This PR changes the announcement of what
nullattribute values will mean in 3.x.The plan towards fixing #3261 is as follows:
In 2.x:
nullvalues are treated astrue, giving the deprecation noticetrueto keep the current behavior and make the notice go awayIn 3.x:
nullvalue (as well asfalse) omit the attributearia-*attributes with afalsevalue will trigger a deprecation notice, saying that it will be"false"in 4.0. To omit thearia-*attribute,nullshall be used and will make the notice go away.In 4.x:
aria-*, convertfalseto"false", to be in line withtrue => "true", closing [TwigComponent] Reconsider behaviour for(bool) falsevalue for ARIA attributes #3261.