Skip to content

[DependencyInjection][Routing] Deprecate XML configuration format #60568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: 7.4
Choose a base branch
from

Conversation

MatTheCat
Copy link
Contributor

@MatTheCat MatTheCat commented May 28, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? no
Deprecations? yes
Issues Fix #60200
License MIT

From #60560 (comment)

@MatTheCat MatTheCat requested a review from dunglas as a code owner May 28, 2025 07:16
@carsonbot carsonbot added this to the 7.4 milestone May 28, 2025
@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch 2 times, most recently from 2169ea4 to c5c98cb Compare May 28, 2025 09:06
@MatTheCat MatTheCat requested a review from yceruto as a code owner May 28, 2025 09:06
@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch from c5c98cb to e7849d9 Compare May 28, 2025 10:04
@MatTheCat MatTheCat requested a review from chalasr as a code owner May 28, 2025 10:04
@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch 2 times, most recently from f21defc to d725030 Compare May 28, 2025 10:27
@stof
Copy link
Member

stof commented May 28, 2025

Let's wait on the discussion on the issue to know what should actually be deprecated. This PR deprecates a lot more than what was mentioned in the issue it fixes.

@carsonbot carsonbot changed the title Deprecate XML configuration format [DependencyInjection][Routing][Serializer][Validator] Deprecate XML configuration format May 28, 2025
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀
Some test cases need some love.

public function testInlineServicesAreNotCandidates()
{
$this->expectUserDeprecationMessage('Since symfony/dependency-injection 7.4: XML configuration format is deprecated, use YAML or PHP instead.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we rewrite this to use another format instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test looks like it’s XML specific: #24491

@@ -32,6 +33,8 @@

class XmlDumperTest extends TestCase
Copy link
Member

@nicolas-grekas nicolas-grekas May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

side question: do we want to keep the xml dumper?
it's used by some external tools AFAIK, so likely yes

Copy link
Contributor Author

@MatTheCat MatTheCat May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have commands loading such XML dumps, like debug:container. Not sure what we should do about that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #60597 which might set us free from this concern.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the XML dump of the container is used by every static analysis Symfony Plugin I know, including PHPStan, PhpStorm's Symfony plugin and Psalm. I don't think we can get rid of it (especially considering PhpStorm's plugin isn't written in PHP so probably can't parse the PHP serialized data nicely).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #61280 to remove the dependency on ext-xml for the XmlDumper

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumping console data in XML must be kept.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XmlDescriptor are not affected, so we're good.

@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch 3 times, most recently from ded957c to 55ac82e Compare May 30, 2025 10:02
@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch 2 times, most recently from 703118b to 8a4b6ca Compare June 5, 2025 08:40
@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch 3 times, most recently from 278edc5 to e01cb8e Compare June 17, 2025 13:31
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Let's get rid of XML and ext-xml.

@xabbuh
Copy link
Member

xabbuh commented Jul 31, 2025

I am not really sure we need to go thus far. At least for the Validator component I can tell that there has been nearly zero maintenance work during the last years for the XmlFileLoader while deprecating it would mean that bundles/applications using it would have to make the switch (I have no idea how widespread that's used though).

@nicolas-grekas
Copy link
Member

Let's split Validator and Serializer in a separate PR?

@mbabker
Copy link
Contributor

mbabker commented Jul 31, 2025

At least for the Validator component I can tell that there has been nearly zero maintenance work during the last years for the XmlFileLoader while deprecating it would mean that bundles/applications using it would have to make the switch (I have no idea how widespread that's used though).

Sylius uses XML files for its validator config, as one visible OSS example.

@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch from 072f096 to b374b96 Compare July 31, 2025 13:28
@MatTheCat MatTheCat changed the title [DependencyInjection][Routing][Serializer][Validator] Deprecate XML configuration format [DependencyInjection][Routing] Deprecate XML configuration format Jul 31, 2025
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still 👍

@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch from 87c2578 to c225359 Compare August 2, 2025 16:33
@derrabus
Copy link
Member

derrabus commented Aug 2, 2025

I'm with @xabbuh on this one. I understand that we don't want to require ext-dom anymore and that's great. But if someone likes using XML config files, why shouldn't we allow it optionally if the maintenance overhead is barely measurable?

@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch from ade2761 to 6527895 Compare August 3, 2025 11:57
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Aug 3, 2025

Validator and Serializer have been split out, so this PR is now only about routing and DI. From them, deprecating makes sense to me: maintenance is still difficult because of XSD, but the most important is the overhead on the doc and for bundles. When there was no PHP format, XML was OK. But now that we have it, there's no point for writing XML for that anymore.

@MatTheCat MatTheCat force-pushed the deprecate-xml-loaders branch from 6527895 to aca2986 Compare August 3, 2025 12:02
@MatTheCat
Copy link
Contributor Author

( Serializer and Validator can be removed from this PR.)

@wouterj
Copy link
Member

wouterj commented Aug 3, 2025

I have a hard time following this initiative to be honest. Given the impact on the ecosystem can be quite significant, I think it's a good idea to make a clear high level plan grounded by clear issues with the current system.

#60200 has a very clear problem statement: maintaining the FrameworkBundle's XSD file has proofed to be a burden, often forgotten when adding new config options.

I very much agree with @stof's comment that this is different for maintaining bundle configuration than maintaining XML loaders for routing, validation, serializer, etc. These formats have proofed to be quite static over the time, requiring less changes to their XSD files and thus less burden from this problem.
I fail to see why we split out serializer and validation from this PR, but keep routing and service definitions in. That seems highly inconsistent to me and makes it pretty unpredictable for users to know where XML can be used, and where it can't.


Further on in the discussion, other problems have been mentioned like XML being a documentation burden and XML special treatment like fixXmlConfig() is a burden. Do we have validation of these problem statements, are they indeed causing unreasonable busy work for maintainers/documentors to outweigh the deprecation impact for the ecosystem?
Note that fixXmlConfig(), despite its name, is also used by the other loaders (allowing you to type role: ROLE_ADMIN instead of roles: [ROLE_ADMIN] in Yaml for instance). I believe the new PHP object format even requires it. This partially invalidates that problem statement's relevance (it's also not deprecated in this PR, so irrelevant in the discussion altogether).


Last but not least, the alternatives are quite the moving target at the moment. Work on the JSON schema for Yaml has been stalled for some time (#59620). And the PHP config objects might be replaced with array shapes (#58771) some time in the future. I'm not sure it's great to force the ecosystem (mostly talking about the open source bundles and projects like drupal and sulu) to such new and moving formats, instead of relying on the very mature and stable XML format.
This is apart from all these alternatives relying on auto generating code in the project. Despite the formats being very cool, this has proofed to be a problem point in many occasions (it's hard to discover and many users try to find the classes in vendor/, static analysis tool can't find the code in var/, it requires booting up the Symfony kernel to generate them, etc.).

All in all, I think the timing is a bit off and we must first guarantee the new formats are stable and remove some of the troublesome points before deprecating older formats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Deprecate and remove support for semantic XML configuration