Injection Mistakes That Hurt Later
Four dependency injection habits that pass review by accident and fail in production: fat constructors, cycles, and the object manager.
public function __construct(private readonly ItemFactory $itemFactory) { }Injection mistakes rarely fail on the day you write them. These four surface later: in review, on a production server, or in the next person's week.
A constructor that keeps growing
Ten injected dependencies is not a configuration problem, it is one class doing several jobs. Split it. Core bundles common block dependencies into Magento\Framework\View\Element\Template\Context, but inventing your own context object only hides the count.
A repository inside an entity
A repository is the service that loads and saves one kind of entity, covered in Stage 3. Injecting one into the entity it manages pulls the whole persistence graph into an object that gets built by the thousand, and it invites a cycle: the repository builds the entity, the entity asks for the repository. Keep lookups in the service or controller that calls you.
Circular dependencies
Two classes that need each other raise LogicException: Circular dependency: A depends on B and vice versa. That check exists only in the developer-mode factory. In production the same pair recurses until PHP runs out of memory. Break the cycle by extracting what both need, or inject a generated \Proxy on one side so the object is not built until a method is called.
The object manager itself
Injecting ObjectManagerInterface, or calling ObjectManager::getInstance(), hides every real dependency from the compiler, from dev:di:info and from tests. Use a generated factory for anything you must create at runtime. After changing a constructor, delete generated/code and run bin/magento setup:di:compile so the generated classes match.
Example
<?php
// app/code/Vendor/Module/Model/Export/Runner.php
declare(strict_types=1);
namespace Vendor\Module\Model\Export;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Store\Model\ScopeInterface;
use Psr\Log\LoggerInterface;
class Runner
{
private const XML_PATH_ENABLED = 'vendor_module/export/enabled';
public function __construct(
private readonly ScopeConfigInterface $scopeConfig,
private readonly WriterFactory $writerFactory,
private readonly LoggerInterface $logger
) {
}
public function run(string $storeCode): int
{
$enabled = $this->scopeConfig->isSetFlag(
self::XML_PATH_ENABLED,
ScopeInterface::SCOPE_STORE,
$storeCode
);
if (!$enabled) {
return 0;
}
// A writer is created per store, so it is not injectable.
// A generated factory creates it; the object manager is never
// called directly.
$writer = $this->writerFactory->create(['storeCode' => $storeCode]);
$this->logger->info('Export started', ['store' => $storeCode]);
return $writer->write();
}
}When to use it
- Review a third-party module before installing it and find the object manager in a constructor.
- Explain a 500 in production from code that runs fine in developer mode.
- Add a dependency to a class other modules already extend, without breaking them.
- Cut the object graph a controller builds when the expensive collaborator is needed on one branch only.
More examples
Breaking a cycle with a proxy
The proxy is generated, so there is no file to write: it stands in for Runner and builds the real object on the first method call, which is after both constructors have finished. Reach for it to unblock yourself, then ask whether the cycle is telling you that one shared piece belongs in a third class. Apply it with bin/magento cache:clean config, and with setup:di:compile before production.
<?xml version="1.0"?>
<!-- app/code/Vendor/Module/etc/di.xml -->
<!--
Developer mode, before:
LogicException: Circular dependency: Vendor\Module\Model\Export\Runner
depends on Vendor\Module\Model\Export\Notifier and vice versa.
Production mode, before: Allowed memory size exhausted, no message.
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
<type name="Vendor\Module\Model\Export\Notifier">
<arguments>
<argument name="runner" xsi:type="object">Vendor\Module\Model\Export\Runner\Proxy</argument>
</arguments>
</type>
</config>Adding a constructor argument without breaking other modules
Another module may already subclass this class and call parent::__construct with the old argument list. Appending a nullable parameter with a null default keeps that code working, and the fallback fills it in. Magento core uses exactly this shape, for example for the AreaList argument of Magento\Developer\Console\Command\DiInfoCommand. Do not use it for a brand new class, and never insert the parameter in the middle.
<?php
// app/code/Vendor/Module/Model/Export/Runner.php
declare(strict_types=1);
namespace Vendor\Module\Model\Export;
use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\ObjectManager;
use Psr\Log\LoggerInterface;
use Vendor\Module\Model\Export\Stats\Collector;
class Runner
{
private Collector $collector;
public function __construct(
private readonly ScopeConfigInterface $scopeConfig,
private readonly WriterFactory $writerFactory,
private readonly LoggerInterface $logger,
?Collector $collector = null
) {
// The one sanctioned ObjectManager call: a new dependency added
// to a released class, last in the list, nullable, defaulted.
$this->collector = $collector ?? ObjectManager::getInstance()->get(Collector::class);
}
}What to inject instead of the object manager
Keep this shape in mind so you recognise it in a module you inherit. The rule is one sentence long: inject an interface for a shared service, inject a generated factory for an object you create per call, and let the constructor be the complete list of what the class needs. A class written this way can be tested with mocks and reconfigured with di.xml; the version above can be neither.
<?php
// app/code/Vendor/Module/Model/Export/Rejected.php
declare(strict_types=1);
namespace Vendor\Module\Model\Export;
use Magento\Framework\ObjectManagerInterface;
class Rejected
{
public function __construct(
private readonly ObjectManagerInterface $objectManager
) {
}
public function run(string $storeCode): int
{
// Nothing here is visible to dev:di:info, to a preference, or
// to a unit test. Any class can be swapped for any other at
// runtime, and nobody reading the constructor can tell.
$writer = $this->objectManager->create(Writer::class, ['storeCode' => $storeCode]);
return $writer->write();
}
}
// Replace it with two declared dependencies:
//
// public function __construct(
// private readonly WriterFactory $writerFactory,
// private readonly LoggerInterface $logger
// ) {
// }
Discussion