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

[RFC] [DI] Functional Service Support #29997

Closed
ragboyjr opened this issue Jan 26, 2019 · 10 comments · Fixed by #34881
Closed

[RFC] [DI] Functional Service Support #29997

ragboyjr opened this issue Jan 26, 2019 · 10 comments · Fixed by #34881
Labels
DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@ragboyjr
Copy link
Contributor

ragboyjr commented Jan 26, 2019

A somewhat common pattern seen among the PHP community is making final classes with one method __invoke like so:

final class DoSomething {
  public function __constructor(/* args */) {
     // set args
  }
  public function __invoke(): void {
    // do something
  }
}

Semantically, this is equivalent to the following done with functions and closures:

function doSomething(/* args */) {
  return function() use (/* args */): void {
    // do something
  };
}

I'd like to discuss ways in which we could make the latter (functional service) easier to use in a symfony application.

The former pattern I think emerged in PHP rather than the latter due to PSR-4 composer autoloading defaults and the fact that most autowired di frameworks (like SF DI) use the class name to wire dependencies.

However, using the function/closure definition is much more terse, and allows you to easily include multiple related services into one file.

Example

With SF's new php fluent container configuration, i've been able to make use of functional services relatively pain free with the help of a few helper functions and abusing the php lexer.

// src/services.php

namespace App;

const serviceA = serviceA::class;
function serviceA(callable $serviceB) { // $serviceB is autowired
   return function() use ($serviceB) {}; // does stuff
}

const serviceB = serviceB::class;
function serviceB(Psr\Log\LoggerInterface $logger) {
     return function(): int {}; // does stuff
}
// src/util.php

namespace App;

/**
 * Registers a service that is a function which returns a closure. If a suffix is applied, the function name is suffixed with `.{suffix}`
 */
function fnService(AbstractServiceConfigurator $container, string $fnName, ?string $suffix = null, ?string $bindName = null): ServiceConfigurator {
    $serviceId = $fnName . ($suffix === null ? '' : '.' .$suffix)
    $container->set($serviceId, 'Closure')->factory($fnName);
    if (!$bindName) {
        $parts = explode('\\', $fnName);
        $bindName = end($parts);
    }

    $container->bind($bindName, ref($serviceId);
}
// config/services.php

use App;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use function App\fnService;
use function Symfony\Component\DependencyInjection\Loader\Configurator\ref;

return function($configurator) {
  $container = $configurator->services()
    ->defaults()->autowire()->autoconfigure()->private();

  fnService($container, App\serviceA);
  fnService($container, App\serviceB);
};

Desired Outcomes

I'd be real stoked to see if we could do something like $container->fn() which is basically my implementation of fnService.

I'm also very open to ideas of how we could possible make this even more robust.

As far as dealing with the pain of having to register non-psr4 files into the files section of the composer.json, that can be mitigated with a composer plugin like: https://github.com/krakphp/php-inc.

@xabbuh xabbuh added DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Jan 30, 2019
@xabbuh
Copy link
Member

xabbuh commented Jan 30, 2019

Same as #28992?

@stof
Copy link
Member

stof commented Jan 30, 2019

The main drawback of using functions rather than classes is that you cannot autoload them. Anything you put in the files section of composer (or that you add in this php-inc plugin, as it works the same) is always loaded at the beginning of the request (see the drawback section of the plugin). Loading unused code (even optimized by OPCache) has a performance cost (this is why the compiled container now generates separate files for the instantiation of most services instead of generating a single huge file).

Thus, I don't even think that a named function returning a closure having access to the dependencies passed to it is actually more readable than an invokable class (and it still does not prevent you from finding a name for your factory function).

@stof
Copy link
Member

stof commented Jan 30, 2019

@xabbuh no, not the same. In this request, the service itself is an anonymous callable, created by a named factory. #28992 is requested the support of anonymous functions as factory for services.

@ragboyjr
Copy link
Contributor Author

@stof Understood, your points are valid, this feature might make more sense if php had the ability to autoload functions. Wanted to bring it up in case this made sense to others as well, or if possibly there were better ideas to support such a thing.

@ghost
Copy link

ghost commented Feb 2, 2019

@stof @ragboyjr

What do you think of this rough idea?

What one could do is have a macorable class that lets you easily bind different functions together and have some sort of invocation hook. Laravel has a dead simple implementation of it here:

Laravel Macorable Trait

You could extend this idea a number of ways: for instance:

Parsing out macros from a macro.yaml file using expression-language. You might get some good caching out of this (unsure if that would be supported )

You could also have it be an entry point where you can alias its static methods to its loaded macros (I believe this would be possible via something like

function _t() {
    call_user_func_array(array('TranslationClass', '_t'), func_get_args());
}

)

Lastly, What about simply using require or I think more ideally require_once a functions.php at runtime (this you can do and functions can be namespaced to avoid collisions) like when one loads bundles but just part of the kernel process (don’t see how this would be an issue per say) this of course would not need a macro class it would just be a sane hook into the loading system

I’m interested in something like this for passing utitlies without having to go through and get a ton of classes created for the purpose

@jkufner
Copy link

jkufner commented Feb 3, 2019

It is possible to use autoloader with functions. The trick is to use a class :)

class DoSomething {
    public static function createFactory(/* args */) {
        return function() use (/* args */): void {
            // do something
        };
    }
}

However, I'm not sure if it is a good idea. Passing callables around is problematic because PHP cannot type-hint them properly (it is not possible to specify arguments in the type hints). I think that the little bit of verbosity is not that bad regarding readability and understandability of the code. While the factory may look more compact, code using such a factory is likely to be more difficult to understand.

@ragboyjr
Copy link
Contributor Author

ragboyjr commented Feb 8, 2019

@ProtonScott I'm not 100% sure how your example would work, can you give some specific examples?

@ghost
Copy link

ghost commented Feb 8, 2019

@ragboyjr Absolutely I'd be happy to.

As a side note: Composer Can Autoload Functions so I'm not sure if that makes any difference here. There's no real reason we could not state in the documentation that to use a function as a service (FAAS!) that you must include it in the composer.json,

I think from there it would be much more simple to work with.

Anyways, as a discreet alternative, using the Macroable class (and I'm assuming, for the sake of argument, the Macroable class would be included in some kind of component or the symfony/framework-bundle maybe) you could have a functions.php that looks like this:

$funcs = [
  'callback' => function (string $echo) {
    echo $echo;
  },
];

return Symfony\Component\Macroable\Macroable::addFuncs($funcs); // this could return a new instance of the Macorable class with all the macros set or something.

Now, you have an array of functions that are defined (the name of the function being the key),

So that can be handled, more or less like it is here

Now thanks to something like this being a class that handles the registered functions at runtime (not unlike how the Kernel handles bundles), you could do something like this:

services:
  App\Functions\UsefulFunc:
     # ...
     calls:
       - method: callback
       - arguments: 'Hello World'

This is a very basic example of what I was thinking (it could use a little more fleshing out, but I hope you get the basic idea of what I am going for here)

Effectively you just create a function registry.

@ragboyjr
Copy link
Contributor Author

ragboyjr commented Feb 8, 2019

Ah, ok, i think I understand what you're talking about. App\Functions\UsefulFunc would be a faux class that is handled via the Macroable class which resolves to the callback closure defined in the $funcs array.

Is my understanding correct?

@ghost
Copy link

ghost commented Feb 8, 2019

@ragboyjr Yup (an alias basically), an alternative of course would be to just call it directly against the Macro class like so:

services:
  app.some.service:
     # ...
     calls:
       - method: Symfony\Component\Macroable\Macroable::callback
       - arguments: 'Hello World'

or you could use an interface to define it (for contracts)

services:
 App\Contract\CallBackInterface:
     # ...
     calls:
       - method: Symfony\Component\Macroable\Macroable::callback
       - arguments: 'Hello World'

I also don't see why this wouldn't be valid either:

services:
    App\Contract\CallbackInterface:
       macro: Symfony\Component\Macroable\Macroable::callback # so we get the function as the service, arguments should always be optional as the function may be a utility that needs to be called elsewhere in the application.

or maybe:

services:
    App\Contract\CallbackInterface:
       bind:
            macro: Symfony\Component\Macroable\Macroable::callback

There is a ton of room for flexibility here.

I would of course be curious if there is anything preventing the autoload via composer from calling in directly as I linked to before. That would certainly simplify this whole affair even further, because then you can just use a namespaced function no different than a class. You would just need to typehint Closure or callable.

e.g.

/// ... composer info
"autoload": {
    "files": ["src/Functions/Functions.php"]
}
/// ... composer info

Now that its loaded its all nice and namespaced you could do:

services:
  App\Functions\SuperAwesomeFunc:
      macro: true # so we know that we want this to be autowired, or however we want to handle the resolution

or maybe

services:
  App\Functions\Contract\CoolJsonFunctionInterface:
     class: App\Functions\json_func # treat the function as if it is a class? I feel like a discreet name is better

The advantages of autoloading the functions as I see it right now:

  • Easier to do, since composer allows for it, and you just have to point it at an array of files
  • The location of the files is more transparent
  • You can hook into whatever you can with the composer autoloader in the same way (not sure if this matters for symfony or not)
  • I would imagine this makes caching a bit easier

The disadvantage:

  • Entry points may not be defined as cohesively
  • There may be some caveats to keep in mind (like, if you have an interface in the same folder, will that cause any issues on the autoload side for autowiring. I must admit I'm not an expert on this, so I might be barking up the wrong tree with that)
  • I'm not sure if there is any real additional overhead doing it this way or not
  • Its entirely possible this would severely complicate type hinting as a guard against autowiring the wrong thing, on the other hand, this should really be up to the developer to decide

Using a Macro Class to load them up from a single functions.php file (similiar to bundles.php):

  • The developer would have to explicity define the functions there, and/or explicilty require additional funcs (maybe have a config/functions folder that the kernel doesn't try to load configuration files from)
  • you could certainly have more control over the format (like the array style)
  • Beyond accommendating it with a new file/folder layout, everything else more or less hooks up the way I would expect, except maybe adding a macro option for services ( in which case the type hint needs to be the Macroable Class)

downside:

  • biggest obvious one I can think of is the type hints won't mean much, so developers will need to be (and I hope they already are!) very deliberate in making sure the paramater is annotated in the dock block with the specific "method" that will get called
  • to that note, autocomplete may not be great in some editors since you can't annotate the Macro class itself

I don't think any of this is huge in terms of hurdles, and I'm leaning closer to being able to autoload/autowire as long as the function file(s) are includedin the composer.json and namespaced.

One other quick thought:

you could make the macro class abstract, and extending that and adding all the functions to it in a pre-defined way would possibly be a good way around some issues as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
4 participants