Skip to content

Conversation

@HLeithner
Copy link
Contributor

@HLeithner HLeithner commented Jul 13, 2021

This is just a request for comments PR.

This implementation is a discussion starter for DI in fx, it replaces the $loop constructor variable with the ContainerInterface. I know this is a breaking change but keeping several legacy layers before a public release doesn't makes much sense and increases the maintenance effort.

I implemented also 2-3 example usages for the container.

  1. loop
  2. logger
  3. socketServerContext

The loop entry is just a simple replacement for the existing $loop parameter.

The logger entry is a LoggerInterface defined by PSR-3 standard. It's used in the log function. Additional to this the request Logger supports now a logger attribute from the request. This can be used for context logging.

The socketServerContext is an array for the socketServer with on addition, it can hold a key named uri which is used for the socketServer uri parameter. This allows us to define a custom IP and port. Also we can use TLS and set custom parameters for the socketServer.

It adds 2 dependencies

  1. php-di PSR-11
  2. psr/log PSR-3

php-di brings a complete dependency injection package but required for other packages is only the ContainerInterface
psr/log brings only the PSR-3 LoggerInterface

Both dependencies are optional for applications based on fx.

Missing

  • Tested in a 3rd party application
  • Documentation
  • Tests
  • Think about it

Just as remark this is just a discussion PR and should be a proof of concept.

Support SocketServer context
Support LoggerInterface
@HLeithner HLeithner changed the title [RFC] Container support [RFC] PSR-11 Container support Jul 13, 2021
@HLeithner
Copy link
Contributor Author

@clue any thoughts about this?

@clue clue added help wanted Extra attention is needed new feature New feature or request question Further information is requested labels Aug 9, 2021
@clue
Copy link
Owner

clue commented Aug 9, 2021

@HLeithner Thank you very much for bringing this up, I think this is an excellent starting point to discuss a DIC integration in X!

I like the direction this is taking and agree that adding an optional DIC as the first argument makes perfect sense. I also agree that this should allow swapping out internal implementation details such as the socket server and eventually the logger.

To move things forward, I would suggesting starting with a smaller changeset and possibly remove the logging aspect from this PR to fully focus on the DIC part. Injecting a custom logger makes perfect sense, but I think it makes sense to address separate implementation approaches in a separate PR.

Once the DIC is in, I'm also really looking forward to using this for controller instantiation to automatically inject required dependencies and only instantiate controllers when they're actually needed. This is going to be an interesting challenge to make sure this will be done on demand behind existing SAPIs and probably on-load when using the built-in webserver.

Full DIC integration is definitely on the mid-term roadmap and this is something I'm planning to look into in the next few weeks/months. If you feel this is something you can contribute to, make sure let me know 👍

@HLeithner
Copy link
Contributor Author

To move things forward, I would suggesting starting with a smaller changeset and possibly remove the logging aspect from this PR to fully focus on the DIC part. Injecting a custom logger makes perfect sense, but I think it makes sense to address separate implementation approaches in a separate PR.

Ok will remove the logger and move it to it's own PR.

Once the DIC is in, I'm also really looking forward to using this for controller instantiation to automatically inject required dependencies and only instantiate controllers when they're actually needed. This is going to be an interesting challenge to make sure this will be done on demand behind existing SAPIs and probably on-load when using the built-in webserver.

So you would like to have some magic that boot a controller (or mvc "component") when required in a request. Isn't this done already by fast-route? I didn't checked the code but if I use

        $app->get(
            '/messages',
            'My\Messages\Controller::getMessages'
        );

I would expect the Controller class will only loaded when /messages is requested. ymmv

Full DIC integration is definitely on the mid-term roadmap and this is something I'm planning to look into in the next few weeks/months. If you feel this is something you can contribute to, make sure let me know 👍

Of course I will help on this topic.

For this PR, would you like a b/c layer for $loop in the App constructor?

What's about the socket Server Context? should be move to it's own PR too?

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

Labels

help wanted Extra attention is needed new feature New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants