Skip to content

Modernize bundle structure: AbstractBundle, PHP config, exception types, dependency constraints #30

Description

@loevgaard

Collecting the structural items in one issue. Most of it is already known (the XML loader deprecation is noted in the extension) and tied to Symfony 8 support.

  1. AbstractBundle and PHP service config. XML DI configuration is deprecated in Symfony 7.4 and removed in 8.0. Moving to Symfony\Component\HttpKernel\Bundle\AbstractBundle with configure(DefinitionConfigurator $definition), loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder) and config/services.php removes the separate Extension/Configuration classes and the Resources/ directory. AbstractBundle expects the modern layout (config/ at the bundle root), so this pairs with moving src/Resources/config to config/.
  2. Exception types. SetonoMetaConversionsApiExtension::load() uses Webmozart\Assert\Assert::true()/keyExists() (src/DependencyInjection/SetonoMetaConversionsApiExtension.php:39-44), which throw InvalidArgumentException. Symfony convention for "bundle X must be registered" is \LogicException, and configuration errors should be InvalidConfigurationException. The current integration test even asserts on \InvalidArgumentException. With AbstractBundle the check becomes $builder->hasExtension('setono_tag_bag') inside loadExtension().
  3. Configuration::isTagBagBundleInstalled() (src/DependencyInjection/Configuration.php:38) makes the configuration schema depend on installed packages, so config:dump-reference differs between installations. Reasonable trade-off, but state it in the README next to the client_side.enabled default. The same InstalledVersions::satisfies() check is duplicated in the extension; keep one.
  4. Composer constraints. symfony/event-dispatcher-contracts: ^2.5 || ^3.0: 2.x pairs with Symfony 5.4, which is no longer supported; ^3.0 is enough. symfony/messenger is required even for client-side-only installations; consider moving it to suggest with a compile-time check when server_side is enabled.
  5. prepend() should go away entirely (prepend() replaces the application's default Messenger bus #12).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions