core icon indicating copy to clipboard operation
core copied to clipboard

PHP 8 named arguments support

Open fadrian06 opened this issue 2 years ago • 4 comments

PHP named arguments helps in some situations for cleaned code, it locks library maintainers to not change the argument names so easily without breaking changes, but php 8 must have support too...

public function test_static_route(): void {
  Flight::request()->url = '/test';

  $route = Flight::route(
    pass_route: true,
    alias: 'testRoute',
    callback: function () {
      echo 'test';
    },
    pattern: '/test'
  );

  self::assertInstanceOf(Route::class, $route);
  self::expectOutputString('test');
  Flight::start();
}

fadrian06 avatar Feb 23 '24 03:02 fadrian06

Ok to start the tests for php 8 are excluded from the main suite, I run the tests of that specific class by hand

fadrian06 avatar Feb 23 '24 05:02 fadrian06

Calling the app directly instead of __callStatic does make sense because it would help the static analyzer

fadrian06 avatar Feb 23 '24 05:02 fadrian06

20240223_015803.jpg

20240223_015800.jpg

fadrian06 avatar Feb 23 '24 06:02 fadrian06

I have several ideas to solve it but I don't know how difficult it is to implement it and keep everything working as is.

  • Extract shared functionalities into traits (it violates SOLID everywhere, although the Flight class does it but in an elegant way delegating everything to the Dispatcher, the Loader and the Engine)

  • It could be by extracting everything into a single trait that is automatically generated with an automated script, so with each docblock that is added, the static method will be created

fadrian06 avatar Feb 23 '24 06:02 fadrian06

I feel that the PR has covered something else related to the container, but I felt that I should fix it before dealing with other things.

fadrian06 avatar Mar 13 '25 04:03 fadrian06

Changes about the container

Before

If you install a psr11 compatible container, and pass it to Flight::registerContainerHandler($container), works only if a class you have previously registered in the container.

For example previously

$container = new Container;
$container->set(DatetimeImmutable::class, fn() => new DatetimeImmutable);

Flight::registerContainerHandler($container);

// later in some controller
function __construct(DatetimeImmutable $dt) {}

Works. Only because you previously registered that class

Modern containers like illuminate/container or flightphp/container resolves that classes automagically because it has optional parameters, others classes doesn't have constructor parameters, container resolves them too...

[!NOTE] But previously fails, the solution to use that automagically class resolution was Flight::registerContainerHandler([$container, 'get']); or Flight::registerContainerHandler($container->get(...)); in php 8

But now:

After (using flightphp/container or another psr11 container with autowiring)

$container = new Container;
-$container->set(DatetimeImmutable::class, fn() => new DatetimeImmutable);

-Flight::registerContainerHandler([$container, 'get']);
+Flight::registerContainerHandler($container);

// later in some controller
function __construct(DatetimeImmutable $dt) {}

And works too, because, flightphp/container resolves a DatetimeImmutable object because, all its constructor parameters are optional.

fadrian06 avatar Mar 13 '25 04:03 fadrian06