core icon indicating copy to clipboard operation
core copied to clipboard

update 'dynamic' routing based on names of content types and taxonomies

Open simongroenewolt opened this issue 2 years ago • 3 comments

This change moves content-type and taxonomy routing config from Kernel + annotations to a RouteLoader service class. By doing so it removes (some of) the dependencies of Kernel.php on config parsers, the use of these parsers by Kernel.php can be problematic/confusing for developers working on or using Bolt because it removes the possibility to override (parts of) the Config.php class, for example during testing. As the Config.php class is itself a service, it can not be used by the kernel (you cannot use services while configuring the service container)

simongroenewolt avatar Feb 19 '22 20:02 simongroenewolt

Hmm, the phpunit errors do not look like they are related to my changes, or are they?

Update: there were related to my changes, pushed a fix.

simongroenewolt avatar Feb 19 '22 20:02 simongroenewolt

I've been thinking a bit more about the general Bolt configuration setup, and while I do think this PR could make sense, I'm wondering if it is the right approach after all. This is because of on the way Config.php is used, being injected as a service, vs. the 'general' way configuration is handled by symfony and related packages/extensions. If I understand correctly most bundles handle config parsing in the container building phase (via an Extension class), which is more or less the use in the same phase as I've removed in this PR. My guess is that when you decided on the bolt config setup there probably were good reasons to make it like it is now - it would be interesting to know more about the reasons why it is like it is.

simongroenewolt avatar Feb 20 '22 12:02 simongroenewolt

My guess is that when you decided on the bolt config setup there probably were good reasons to make it like it is now - it would be interesting to know more about the reasons why it is like it is.

It is the way it is now, because I didn't know how else I could (programmatically) add the requirements to the routing. Personally, I think this is an improvement, because it removes black magic from Kernel. It might look like it's more code than before, but if you take into account a large part of the new code is comments, it seems to be about the same.

I'm 👍 on this

bobdenotter avatar Feb 21 '22 16:02 bobdenotter