-
Notifications
You must be signed in to change notification settings - Fork 11.4k
[13.x] RFC: Lazy Services - New #[Lazy] Attribute #55645
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
namespace Illuminate\Container\Attributes; | ||
|
||
use Attribute; | ||
|
||
#[Attribute] | ||
class Lazy | ||
{ | ||
// | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ | |||||||||
use ArrayAccess; | ||||||||||
use Closure; | ||||||||||
use Exception; | ||||||||||
use Illuminate\Container\Attributes\Lazy; | ||||||||||
use Illuminate\Contracts\Container\BindingResolutionException; | ||||||||||
use Illuminate\Contracts\Container\CircularDependencyException; | ||||||||||
use Illuminate\Contracts\Container\Container as ContainerContract; | ||||||||||
|
@@ -1058,8 +1059,16 @@ public function build($concrete) | |||||||||
|
||||||||||
array_pop($this->buildStack); | ||||||||||
|
||||||||||
if (!empty($reflector->getAttributes(Lazy::class))) { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could just directly check this
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this change bring anything beyond syntax? I’m just asking out of curiosity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not much. Technically, a function call is more expensive than a comparison, but this should mostly be optimized away in practice. One slight advantage would be a clear communication that we are working with an array here and don't have to check any structure's emptiness. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @olekjs empty function is a trap for future bugs. empty('0') is true for example. |
||||||||||
$instance = $reflector->newLazyProxy(function () use ($concrete, $instances) { | ||||||||||
return new $concrete(...$instances); | ||||||||||
}); | ||||||||||
Comment on lines
+1069
to
+1071
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could do just
Suggested change
|
||||||||||
} else { | ||||||||||
$instance = $reflector->newInstanceArgs($instances); | ||||||||||
} | ||||||||||
|
||||||||||
$this->fireAfterResolvingAttributeCallbacks( | ||||||||||
$reflector->getAttributes(), $instance = $reflector->newInstanceArgs($instances) | ||||||||||
$reflector->getAttributes(), $instance | ||||||||||
); | ||||||||||
|
||||||||||
return $instance; | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
<?php | ||
|
||
namespace Illuminate\Tests\Container; | ||
|
||
use Illuminate\Container\Attributes\Lazy; | ||
use Illuminate\Container\Container; | ||
use PHPUnit\Framework\TestCase; | ||
|
||
class ContainerLazyTest extends TestCase | ||
{ | ||
protected function tearDown(): void | ||
{ | ||
Container::setInstance(); | ||
} | ||
|
||
public function testConcreteDoesNotThrowsExceptionWithAttribute() | ||
{ | ||
$container = new Container; | ||
$lazy = $container->make(LazyWithAttributeStub::class); | ||
|
||
// No RuntimeException has occurred | ||
// LazyWithAttributeStub behaves like a Lazy Object, but this is not obvious from its type | ||
$this->assertInstanceOf(LazyWithAttributeStub::class, $lazy); | ||
} | ||
|
||
public function testConcreteThrowsExceptionWithoutAttribute() | ||
{ | ||
$this->expectException(\RuntimeException::class); | ||
$this->expectExceptionMessage('Lazy call'); | ||
|
||
$container = new Container; | ||
$container->make(LazyWithoutAttributeStub::class); | ||
} | ||
|
||
public function testConcreteDoesNotThrowsExceptionWithNoLogicConstructor() | ||
{ | ||
$container = new Container; | ||
$lazy = $container->make(LazyWithAttributeStub::class); | ||
|
||
$this->assertInstanceOf(LazyWithAttributeStub::class, $lazy); | ||
|
||
$this->assertSame('work', $lazy->work()); | ||
} | ||
|
||
public function testConcreteDoesThrowsExceptionWithConstructorWithLogic() | ||
{ | ||
$container = new Container; | ||
$lazy = $container->make(LazyWithAttributeLogicStub::class); | ||
|
||
// No RuntimeException has occurred so far | ||
$this->assertInstanceOf(LazyWithAttributeLogicStub::class, $lazy); | ||
|
||
$this->expectException(\RuntimeException::class); | ||
$this->expectExceptionMessage('Lazy call'); | ||
|
||
// Only the call to number() causes a RuntimeException | ||
$lazy->number(); | ||
} | ||
|
||
public function testConcreteThrowsExceptionButNotLazyDependency() | ||
{ | ||
$this->expectException(\RuntimeException::class); | ||
$this->expectExceptionMessage('Parent call'); | ||
|
||
$container = new Container; | ||
$container->make(LazyDependencyWithAttributeStub::class); | ||
} | ||
|
||
public function testConcreteNotLazyDependencyThrowsException() | ||
{ | ||
$this->expectException(\RuntimeException::class); | ||
$this->expectExceptionMessage('Lazy call'); | ||
|
||
$container = new Container; | ||
$container->make(LazyDependencyWithoutAttributeStub::class); | ||
} | ||
} | ||
|
||
#[Lazy] | ||
class LazyWithAttributeStub | ||
{ | ||
public function __construct() | ||
{ | ||
throw new \RuntimeException('Lazy call'); | ||
} | ||
|
||
public function work() | ||
{ | ||
return 'work'; | ||
} | ||
} | ||
|
||
#[Lazy] | ||
class LazyWithAttributeLogicStub | ||
{ | ||
public $number; | ||
|
||
public function __construct() | ||
{ | ||
$this->number = 10; | ||
|
||
throw new \RuntimeException('Lazy call'); | ||
} | ||
|
||
public function number() | ||
{ | ||
$this->number += 10; | ||
} | ||
} | ||
|
||
class LazyWithoutAttributeStub | ||
{ | ||
public function __construct() | ||
{ | ||
throw new \RuntimeException('Lazy call'); | ||
} | ||
} | ||
|
||
class LazyDependencyWithAttributeStub | ||
{ | ||
public function __construct(LazyWithAttributeStub $stub) | ||
{ | ||
throw new \RuntimeException('Parent call'); | ||
} | ||
} | ||
|
||
class LazyDependencyWithoutAttributeStub | ||
{ | ||
public function __construct(LazyWithoutAttributeStub $stub) | ||
{ | ||
throw new \RuntimeException('Parent call'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Laravel 13 will depends on PHP 8.3 as minimum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alrighty, in that case we can consider version 14.x, but any potential merging should wait until 13.x is released - is that correct? :)