From f716004b98ba21f8d2cc50e1b68a285f320dc0b0 Mon Sep 17 00:00:00 2001 From: Mark Shust Date: Wed, 27 May 2026 08:44:09 -0400 Subject: [PATCH] refactor(view-latte,view-twig): move closure to Engine/Environment binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ViewInterface binding was a closure that built the view manually. The closure existed because the engine needs LatteEngineFactory::create() (method call, not autowire). But the closure was at the wrong level — LatteView and TwigView have fully-autowirable constructors. Per Marko's binding guidance ("Prefer simple bindings. Only use closures when autowiring can't express what you need"), the closure should sit on the dependency that genuinely needs the method call (Latte\Engine / Twig\Environment), not bubble up to the parent. After this change: - ViewInterface => LatteView::class (simple binding, autowired) - Latte\Engine::class => closure calling LatteEngineFactory::create() - ViewInterface => TwigView::class (simple binding, autowired) - Twig\Environment::class => closure calling TwigEngineFactory::create() Module tests updated to verify the new binding shape. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/view-latte/module.php | 10 ++- packages/view-latte/tests/ModuleTest.php | 38 +++++------ packages/view-twig/module.php | 10 ++- packages/view-twig/tests/ModuleTest.php | 85 +++++------------------- 4 files changed, 42 insertions(+), 101 deletions(-) diff --git a/packages/view-latte/module.php b/packages/view-latte/module.php index 669061df..31177c38 100644 --- a/packages/view-latte/module.php +++ b/packages/view-latte/module.php @@ -2,19 +2,17 @@ declare(strict_types=1); +use Latte\Engine; use Marko\Core\Container\ContainerInterface; use Marko\View\Latte\LatteEngineFactory; use Marko\View\Latte\LatteView; -use Marko\View\TemplateResolverInterface; use Marko\View\ViewInterface; return [ 'bindings' => [ - ViewInterface::class => function (ContainerInterface $container): ViewInterface { - $engine = $container->get(LatteEngineFactory::class)->create(); - $resolver = $container->get(TemplateResolverInterface::class); - - return new LatteView($engine, $resolver); + ViewInterface::class => LatteView::class, + Engine::class => function (ContainerInterface $container): Engine { + return $container->get(LatteEngineFactory::class)->create(); }, ], ]; diff --git a/packages/view-latte/tests/ModuleTest.php b/packages/view-latte/tests/ModuleTest.php index d9fe2b51..8ad0e934 100644 --- a/packages/view-latte/tests/ModuleTest.php +++ b/packages/view-latte/tests/ModuleTest.php @@ -6,11 +6,10 @@ use Marko\Core\Container\ContainerInterface; use Marko\View\Latte\LatteEngineFactory; use Marko\View\Latte\LatteView; -use Marko\View\TemplateResolverInterface; use Marko\View\ViewInterface; describe('view-latte module.php', function (): void { - test('module.php exists with correct structure', function (): void { + test('it has a bindings array', function (): void { $modulePath = dirname(__DIR__) . '/module.php'; expect(file_exists($modulePath))->toBeTrue(); @@ -22,43 +21,40 @@ ->and($module['bindings'])->toBeArray(); }); - test('module.php binds ViewInterface via factory', function (): void { + test('it binds ViewInterface to LatteView as a simple class mapping', function (): void { $module = require dirname(__DIR__) . '/module.php'; expect($module['bindings'])->toHaveKey(ViewInterface::class) - ->and($module['bindings'][ViewInterface::class])->toBeInstanceOf(Closure::class); + ->and($module['bindings'][ViewInterface::class])->toBe(LatteView::class); }); - test('module.php uses LatteEngineFactory', function (): void { + test('it binds Latte Engine via a closure that calls LatteEngineFactory', function (): void { + $module = require dirname(__DIR__) . '/module.php'; + + expect($module['bindings'])->toHaveKey(Engine::class) + ->and($module['bindings'][Engine::class])->toBeInstanceOf(Closure::class); + }); + + test('the Engine closure resolves the engine via LatteEngineFactory::create()', function (): void { $module = require dirname(__DIR__) . '/module.php'; - // Mock the engine from factory $engine = $this->createMock(Engine::class); - // Mock the LatteEngineFactory $engineFactory = $this->createMock(LatteEngineFactory::class); $engineFactory->expects($this->once()) ->method('create') ->willReturn($engine); - // Mock the TemplateResolverInterface - $resolver = $this->createMock(TemplateResolverInterface::class); - - // Mock the container $container = $this->createMock(ContainerInterface::class); $container->method('get') - ->willReturnCallback(function (string $class) use ($engineFactory, $resolver) { - return match ($class) { - LatteEngineFactory::class => $engineFactory, - TemplateResolverInterface::class => $resolver, - default => throw new Exception("Unexpected class: $class"), - }; + ->willReturnCallback(fn (string $class) => match ($class) { + LatteEngineFactory::class => $engineFactory, + default => throw new Exception("Unexpected class: $class"), }); - // Call the factory closure - $factory = $module['bindings'][ViewInterface::class]; - $view = $factory($container); + $closure = $module['bindings'][Engine::class]; + $result = $closure($container); - expect($view)->toBeInstanceOf(LatteView::class); + expect($result)->toBe($engine); }); }); diff --git a/packages/view-twig/module.php b/packages/view-twig/module.php index 26f903e3..86eacbdd 100644 --- a/packages/view-twig/module.php +++ b/packages/view-twig/module.php @@ -3,18 +3,16 @@ declare(strict_types=1); use Marko\Core\Container\ContainerInterface; -use Marko\View\TemplateResolverInterface; use Marko\View\Twig\TwigEngineFactory; use Marko\View\Twig\TwigView; use Marko\View\ViewInterface; +use Twig\Environment; return [ 'bindings' => [ - ViewInterface::class => function (ContainerInterface $container): ViewInterface { - $engine = $container->get(TwigEngineFactory::class)->create(); - $resolver = $container->get(TemplateResolverInterface::class); - - return new TwigView($engine, $resolver); + ViewInterface::class => TwigView::class, + Environment::class => function (ContainerInterface $container): Environment { + return $container->get(TwigEngineFactory::class)->create(); }, ], ]; diff --git a/packages/view-twig/tests/ModuleTest.php b/packages/view-twig/tests/ModuleTest.php index 8546db83..e7783a87 100644 --- a/packages/view-twig/tests/ModuleTest.php +++ b/packages/view-twig/tests/ModuleTest.php @@ -3,14 +3,13 @@ declare(strict_types=1); use Marko\Core\Container\ContainerInterface; -use Marko\View\TemplateResolverInterface; use Marko\View\Twig\TwigEngineFactory; use Marko\View\Twig\TwigView; use Marko\View\ViewInterface; use Twig\Environment; describe('view-twig module.php', function (): void { - test('it registers a ViewInterface binding', function (): void { + test('it has a bindings array', function (): void { $modulePath = dirname(__DIR__) . '/module.php'; expect(file_exists($modulePath))->toBeTrue(); @@ -19,93 +18,43 @@ expect($module)->toBeArray() ->and($module)->toHaveKey('bindings') - ->and($module['bindings'])->toBeArray() - ->and($module['bindings'])->toHaveKey(ViewInterface::class); + ->and($module['bindings'])->toBeArray(); }); - test('it resolves ViewInterface to a TwigView instance', function (): void { + test('it binds ViewInterface to TwigView as a simple class mapping', function (): void { $module = require dirname(__DIR__) . '/module.php'; - $engine = $this->createMock(Environment::class); - - $engineFactory = $this->createMock(TwigEngineFactory::class); - $engineFactory->expects($this->once()) - ->method('create') - ->willReturn($engine); - - $resolver = $this->createMock(TemplateResolverInterface::class); - - $container = $this->createMock(ContainerInterface::class); - $container->method('get') - ->willReturnCallback(function (string $class) use ($engineFactory, $resolver) { - return match ($class) { - TwigEngineFactory::class => $engineFactory, - TemplateResolverInterface::class => $resolver, - default => throw new Exception("Unexpected class: $class"), - }; - }); - - $factory = $module['bindings'][ViewInterface::class]; - $view = $factory($container); - - expect($view)->toBeInstanceOf(TwigView::class); + expect($module['bindings'])->toHaveKey(ViewInterface::class) + ->and($module['bindings'][ViewInterface::class])->toBe(TwigView::class); }); - test('it injects a configured Twig Environment into TwigView', function (): void { + test('it binds Twig Environment via a closure that calls TwigEngineFactory', function (): void { $module = require dirname(__DIR__) . '/module.php'; - $engine = $this->createMock(Environment::class); - - $engineFactory = $this->createMock(TwigEngineFactory::class); - $engineFactory->expects($this->once()) - ->method('create') - ->willReturn($engine); - - $resolver = $this->createMock(TemplateResolverInterface::class); - - $container = $this->createMock(ContainerInterface::class); - $container->method('get') - ->willReturnCallback(function (string $class) use ($engineFactory, $resolver) { - return match ($class) { - TwigEngineFactory::class => $engineFactory, - TemplateResolverInterface::class => $resolver, - default => throw new Exception("Unexpected class: $class"), - }; - }); - - $factory = $module['bindings'][ViewInterface::class]; - - // The factory closure calls engineFactory->create() — expectation above asserts this - $view = $factory($container); - - expect($view)->toBeInstanceOf(TwigView::class); + expect($module['bindings'])->toHaveKey(Environment::class) + ->and($module['bindings'][Environment::class])->toBeInstanceOf(Closure::class); }); - test('it injects the shared TemplateResolverInterface into TwigView', function (): void { + test('the Environment closure resolves the engine via TwigEngineFactory::create()', function (): void { $module = require dirname(__DIR__) . '/module.php'; $engine = $this->createMock(Environment::class); $engineFactory = $this->createMock(TwigEngineFactory::class); - $engineFactory->method('create') + $engineFactory->expects($this->once()) + ->method('create') ->willReturn($engine); - $resolver = $this->createMock(TemplateResolverInterface::class); - $container = $this->createMock(ContainerInterface::class); $container->method('get') - ->willReturnCallback(function (string $class) use ($engineFactory, $resolver) { - return match ($class) { - TwigEngineFactory::class => $engineFactory, - TemplateResolverInterface::class => $resolver, - default => throw new Exception("Unexpected class: $class"), - }; + ->willReturnCallback(fn (string $class) => match ($class) { + TwigEngineFactory::class => $engineFactory, + default => throw new Exception("Unexpected class: $class"), }); - $factory = $module['bindings'][ViewInterface::class]; - $view = $factory($container); + $closure = $module['bindings'][Environment::class]; + $result = $closure($container); - expect($view)->toBeInstanceOf(TwigView::class) - ->and($view)->toBeInstanceOf(ViewInterface::class); + expect($result)->toBe($engine); }); });