Skip to content
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

Unable to use custom EventSubscriberInterface in depfile since 2.0 as those are prefixed in the phar #1417

Open
janedbal opened this issue May 24, 2024 · 8 comments

Comments

@janedbal
Copy link

I tried upgrading our customized deptrac setup to 2.0 (we have custom baseline implementation) and found that you started prefixing some extension points like EventSubscriberInterface, I believe this only complicates extendability of this library.

app@5fa22c221664:/app$ vendor/bin/deptrac

In RegisterListenersPass.php line 103:
                                                                                                                                            
  [DEPTRAC_202404\Symfony\Component\DependencyInjection\Exception\InvalidArgumentException]                                                 
  Service "baselineViolationHandler" must implement interface "DEPTRAC_202404\Symfony\Component\EventDispatcher\EventSubscriberInterface".  
                                                                                                                                            

Exception trace:
  at /app/vendor/qossmic/deptrac/vendor/symfony/event-dispatcher/DependencyInjection/RegisterListenersPass.php:103
 DEPTRAC_202404\Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass->process() at /app/vendor/qossmic/deptrac/vendor/symfony/dependency-injection/Compiler/Compiler.php:70
 DEPTRAC_202404\Symfony\Component\DependencyInjection\Compiler\Compiler->compile() at /app/vendor/qossmic/deptrac/vendor/symfony/dependency-injection/ContainerBuilder.php:652
 DEPTRAC_202404\Symfony\Component\DependencyInjection\ContainerBuilder->compile() at /app/vendor/qossmic/deptrac/src/Supportive/DependencyInjection/ServiceContainerBuilder.php:84
 Qossmic\Deptrac\Supportive\DependencyInjection\ServiceContainerBuilder->build() at /app/vendor/qossmic/deptrac/src/Supportive/Console/Application.php:64
 Qossmic\Deptrac\Supportive\Console\Application->doRun() at /app/vendor/qossmic/deptrac/vendor/symfony/console/Application.php:162
 DEPTRAC_202404\Symfony\Component\Console\Application->run() at /app/vendor/qossmic/deptrac/bin/deptrac:24
 include() at /app/vendor/bin/deptrac:120

Defined as

services:
  baselineViolationHandler:
    class: ShipMonk\Rules\Deptrac\EventHandler\BaselineViolationHandler
    arguments:
      $baseline: '@baseline'
    tags:
      - { name: kernel.event_subscriber }
class BaselineViolationHandler implements \Symfony\Component\EventDispatcher\EventSubscriberInterface {
    // ...
}
@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented May 24, 2024

I don’t think that the internal dependencies have ever been a public api. Properly a better solution would be to have a official extension point which is provided by Deptrac itself.

another option could be to prefix the dependencies with a static prefix. And people could still use those.

@janedbal
Copy link
Author

I understand. I just think that Deptrac was very nicely customizable and extendable thanks to its event-driven design and the fact you have access to its DIC. That allowed us some neat customizations we needed. And now it got a bit worse :)

@gennadigennadigennadi
Copy link
Member

With using the Symfony event from Deptrac your project adds a dependency. And if Deptracs decides to drop the Symfony stuff your code will break.

long story short: Deptrac should define extension points and document them. Maybe that’s already the case @patrickkusebauch ?

@gennadigennadigennadi
Copy link
Member

By the way, with the new release model, I can release new version quicker! And I will try do release Deptrac more often!

My next goal is to figure out how we deploy the documentation 😅.

@janedbal
Copy link
Author

Yeah, I know the risk and I'm willing to take it. Obviously I'm doing non-standard thing, I just wanna let you know that there are deptrac users that utilize its internal stuff.

@patrickkusebauch
Copy link
Collaborator

I think a nice middle ground would be to have a static prefix for the Symfony Event Dispatcher, something like Deptrac\Symfony\Component\EventDispatcher\EventSubscriberInterface. That way the users can depend on a consistent name to extend that would not break between releases.

@janedbal do you think that would be good enough for your use case?

@gennadigennadigennadi
Copy link
Member

gennadigennadigennadi commented Jun 8, 2024

I’ve merged ‘Deptrac_Internal’ as the namespace prefix for Deptrac a dependencies.

See https://github.com/qossmic/deptrac-src/pull/48/files.
There is not a release wirh the static prefix, so there is still the possibility to change it.

@janedbal
Copy link
Author

If you insist on prefixing, then static one is definitelly better. Thank you

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

No branches or pull requests

3 participants