dunglas / DunglasActionBundle

Symfony controllers, redesigned
https://dunglas.fr/2016/01/dunglasactionbundle-symfony-controllers-redesigned/
MIT License
256 stars 14 forks source link

Add support for more classes #14

Closed GuilhemN closed 8 years ago

GuilhemN commented 8 years ago

Just as a reminder, I see several things that we could automatically register:

To be sure to not have conflicts with existing services, I think we should add a test to only register a class when there is no service corresponding to it. BTW having so many classes automatically registered may introduce performance issues so it would be great to see if it's possible to cache some things or not.

weaverryan commented 8 years ago

I assume this means automatically tag services that have a given "prefix" (not directory)?

I think this is a very interesting idea, but, is there a good way to handle things that need a tag with extra properties? For example, kernel.event_listener? Actually, this is the main one (most-used) that has this problem. A workaround would be to recommend a key like event_susbcriber and point to an EventSubscriber directory. I wouldn't want people to mix subscribers and listeners in that directory and expect the listeners to get tagged automatically.

Also, would it be possible (and is it a good idea) for other bundles to hook into this? For example, it might be interesting if DoctrineBundle could auto-tag doctrine_event_subscriber prefixes.

I'm interested and asking questions... but I'm still not convinced of using the "prefix" (as is done currently with console). Another option is to check all auto-registered auto-wired classes for interfaces and auto-tag those. This would/could only apply if we removed the ability for all bundles to have their services auto-registered, which I think makes more sense anyways.

dunglas commented 8 years ago

@weaverryan I like your last suggestion of adding tag by checking interfaces. I just removed the "autodiscovery" feature in #26. This wasn't possible in early versions of this bundle but thanks to the suggestion of @Ener-Getick to use the Finder Component there is no any interest for this feature now.

GuilhemN commented 8 years ago

I think this is a very interesting idea, but, is there a good way to handle things that need a tag with extra properties? For example, kernel.event_listener? Actually, this is the main one (most-used) that has this problem. A workaround would be to recommend a key like event_susbcriber and point to an EventSubscriber directory. I wouldn't want people to mix subscribers and listeners in that directory and expect the listeners to get tagged automatically.

IMO for extra parameters, people should have their own service definition. If they want their listeners to be automatically tagged, they must implement the subscriber interface.

Also, would it be possible (and is it a good idea) for other bundles to hook into this? For example, it might be interesting if DoctrineBundle could auto-tag doctrine_event_subscriber prefixes.

I like this idea, maybe a solution would be to have a map in the config to add tags in function of the interface the class implement.

I'm interested and asking questions... but I'm still not convinced of using the "prefix" (as is done currently with console). Another option is to check all auto-registered auto-wired classes for interfaces and auto-tag those. This would/could only apply if we removed the ability for all bundles to have their services auto-registered, which I think makes more sense anyways.

:+1:, I'll submit a PR for that.