Skip to content

v3.0.0

Choose a tag to compare

@github-actions github-actions released this 23 Sep 08:24
· 23 commits to main since this release

What's Changed

Upgrading from 2.x. Constraint only for most consumers: composer require parisek/drupal-kit:^3.0. One breaking change is visible on a site, and it needs one command.

hook_file_download() now returns -1 for an anonymous visitor instead of redirecting to /user/login itself. Core answers that with a 403, so a site that serves private files needs a 403 handler:

composer require drupal/r4032login && drush en -y r4032login

Measured across the consuming projects at release time: drupal-base and htdvere use private files and need the module; proficio already had it enabled. The replacement is better than what it removes — r4032login preserves destination, so a visitor returns to the page they asked for after logging in, which the old redirect did not do.

Everything else is invisible to a consumer: the Drupal 10 floor, the #[Hook] migration, hook_runtime_requirements, the FeatureFlags service and the #[Filter] attributes all keep their existing behaviour and public names.

Changed

  • BREAKING: hook_file_download() returns -1 instead of sending a redirect (#141) — the hook used to build a RedirectResponse to /user/login and send it from inside the hook, then return void. The contract is array|int|null (file.api.php:34), and core's own FileDownloadHook returns -1 to deny.

    It looked like it worked because the browser followed a redirect it had already received. The sequence was wrong: FileDownloadController::download() collects what the hooks return, finds nothing, and throws AccessDeniedHttpException — after the response has gone out. Drupal then tried to render a 403 onto a finished request.

    The login redirect is not this library's decision, and the measurement said so plainly. What a site's 403 does belongs to the site, drupal/r4032login already does it properly with destination preserved, and proficio was already running that module while this hook redirected from underneath it. So this is a deletion from the library rather than an exception subscriber written, tested and maintained here.

    Upgrade note. A site that uses private files and does not add drupal/r4032login will show an anonymous visitor "Access denied" where it previously showed the login form. drupal-base and htdvere both use private files and both need the module; proficio already has it. system.site:page.403 is empty on drupal-base, so there is no fallback there today.

    The hook also gets its first test. The old shape could only have been checked as a side effect on a global response, which is part of why none existed. Three kernel cases, mutation-checked, and the interesting one is that an authenticated user must produce an empty result array rather than [NULL]: ModuleHandler::invokeAll() guards each result with isset(), so NULL leaves no trace in the list core scans. Returning 0 instead would grant access — it survives isset(), 0 == -1 is false so core does not deny, and count($headers) then makes FileDownloadController serve the private file with a nonsense header.

  • The remaining core deprecations are cleared, and CI can finally fail on one (#142).

    Five filter plugins move from @Filter annotations to #[Filter] attributes. Core emits a deprecation for every annotation-discovered plugin (AttributeDiscoveryWithAnnotations.php:98, removed in drupal:13.0.0). Plugin IDs are unchanged, so no consumer sees anything.

    drupal_kit.skip_procedural_hook_scan: true. HookCollectorPass then skips this module's procedural file scan entirely. It was unsafe while drupal_kit.install existed — hook_requirements would have stopped registering without a word — and #138 removed the last procedural file.

    New FilterDiscoveryKernelTest, because all five existing filter tests are unit tests that construct the class and call process(). They prove the transformation and say nothing about whether Drupal can find the plugin, so this conversion could have broken discovery with every one of them green — the same gap the #[Hook] migration had. And the failure would be silent: a text format stores filter IDs in config, so an ID that stops resolving does not error, the filter simply stops running and body text renders unprocessed.

Fixed

  • SYMFONY_DEPRECATIONS_HELPER never did anything — symfony/phpunit-bridge is not a dependency of this package and is not in vendor/, so nothing read the variable. Twenty lines of comment weighed weak against disabled against max[total]=0: a careful decision between settings of a mechanism that was not installed.

    That is why #137 — REQUIREMENT_OK removed in drupal:12.0.0 — was found by a code review rather than by CI. The guard was never there; it only looked like it was.

    Replaced with PHPUnit's own: failOnDeprecation on the root element, and ignoreIndirectDeprecations on <source>, whose include list is src/. So a deprecation this module causes fails the build and a deprecation in core or a vendor package does not. Measured before switching it on: 22 PHP deprecations in a full run, 20 of them PHP 8.4 implicit-nullable notices inside mundschenk-at/php-typography and 2 null-array-offset notices inside core. None from src/, so the ratchet costs nothing today.

    Its limit is written into the config rather than left to be discovered: it does not catch a deprecation this module raises with @trigger_error(), which is how Drupal raises them. PHPUnit ignores suppressed deprecations, and the switch that changes that would un-suppress core's too. Verified by mutation — an unsuppressed trigger_error() in src/ exits 1, the same call with @ exits 0.

Changed

  • FeatureFlags is a service (#139) — it was static because the supported range was ^10 || ^11: 10.x has no #[Hook] registration, so every hook was procedural and a service would have had nobody to inject it into. #134 moved the floor and #135 turned all seven hooks into autowired classes, so the reason is gone. Registered as drupal_kit.feature_flags with the config factory injected.

    KNOWN_FLAGS stays a constant. The reason is that it is module-owned data rather than wiring: the container has no business carrying the list of flags this module declares. The list itself earns its keep because schema validation is not runtime enforcement — it runs in tests and in Config Inspector, never on a production read — so it is the only thing between a raw storage write and a "flag" no code here has heard of.

    Two arguments for it that do not hold are recorded in the class docblock, because an earlier version of this entry made both. A constructor argument would not let a project declare its own flags: the service is registered by this module, so changing the argument needs a ServiceProvider, and a ServiceProvider can swap the class and override the constant just as easily. And the other two opt-in patterns do not get typo protection free from PHP — the $params pattern reads keys with isset(), and a subclass that misspells a protected bool simply declares a new property. All three patterns are equally silent about a typo. An independent review found both errors.

    New case: the container really hands out the service. Constructing the class proves the logic and not that a hook can reach it — a misspelled service id would leave every call site fatal on a real site and every test green, because they all build their own instance.

    Nothing in production called FeatureFlags::enabled(), because no flag ships yet (#129 landed the mechanism empty on purpose). That made this the cheapest moment the change will ever have.

    AGENTS.md § Feature flags and README.md both described the static shape. Both are corrected — the AGENTS.md sentence is the one a future flag author follows, so it now also states that #[Autowire(service: 'drupal_kit.feature_flags')] is required: the service id is not the class name, so a bare type-hint fails the container build. RELEASING.md § Public API surface gains the new service id, plus drupal_kit.vite_manifest and drupal_kit.schedule_announcer, which the review found had been missing from that list already.

Changed

  • hook_requirements() becomes hook_runtime_requirements(), and drupal_kit.install is deleted (#137) — the procedural form without #[LegacyRequirementsHook] is deprecated in drupal:11.3.0 and removed in drupal:13.0.0, and the REQUIREMENT_OK / REQUIREMENT_WARNING constants it reported with are deprecated in drupal:11.2.0 and removed in drupal:12.0.0. Those constants were the one thing in this module that would have broken on 12.

    Two tools were looking and neither could see it. phpstan-deprecation-rules has no rule for global constants, so a deprecated const is invisible at level 8. And phpunit.xml.dist sets SYMFONY_DEPRECATIONS_HELPER=weak, so the suite counts core deprecations and never fails on them — which is why the 22 in every run had gone unexamined. Found by an independent doctrine review of #135, not by CI.

    The optional service is now injected instead of looked up. \Drupal::hasService('menu.language_tree_manipulator') asked whether the container knows the name; a nullable constructor argument asks whether this class received it, which is what MenuTreeBuilder actually depends on. Requirements is registered by hand in drupal_kit.services.yml rather than autowired — @? is how an argument says "NULL when missing", no attribute can say that, and Hook.php sanctions manual registration for exactly this case.

    That also makes a branch testable that never was. The old test asked hasService(), and a kernel container cannot answer yes for a service core does not ship, so the RequirementSeverity::OK path had no coverage at all. Handing the class a service is now enough. The replacement test drops the two include_once lines the old one needed to reach core/includes/install.inc for the constants — an enum needs no include.

    drupal_kit.install held nothing else, so it is gone, and with it the last procedural hook in the module. phpstan.neon and phpcs.xml.dist drop their file entries.

Changed

  • The hooks are OOP now: seven #[Hook] classes in src/Hook/, and drupal_kit.module is gone — .claude/rules/drupal/drupal-modules.md has named attribute classes the default for this stack for some time, and this module was the exception because the ^10 floor had no such registration. The floor moved, so the exception goes.

    hook_module_implements_alter() is not ported. It unset this module's entry and re-added it to push form_alter to the end of the list; order: Order::Last states the same thing on the method that needs it. Core deprecated the procedural hook in 11.2.0 and removes it in 12.0.0 unless it carries #[LegacyModuleImplementsAlter], and it raises E_USER_DEPRECATED at every container build until then, so it had to go regardless of this migration.

    The migration removed a dependency nobody could see. hook_locale_translation_projects_alter() read extension.list.module, whose class core marks @internal. \Drupal::service() returns mixed, so the type never had to be written down and nothing ever objected. A constructor argument has to name it, PHPStan reported it immediately, and the hook now takes ExtensionPathResolver — public API, and already what ViteManifest and TypographyExtension in this same module take.

    New: HookRegistrationKernelTest asserts all seven hooks are registered and that the .module file is gone. A #[Hook] class fails by not being found — a renamed method or a dropped attribute leaves code that passes phpcs and PHPStan and is never called, and no other assertion in the suite would notice. Mutation-checked: breaking one attribute fails exactly one case and names the hook.

    InterfaceTranslationsKernelTest changed for a real reason rather than a mechanical one. It swaps the extension service for a stub mid-test, which worked because the procedural hook looked the service up on every invocation. An injected dependency binds once, so the stub now goes in before the first invoke and the hook service is dropped with it. That is the trade the migration makes, written down where the next reader meets it.

Changed

  • BREAKING: Drupal 10 is no longer supported. The floor is ^11.4 (#130) — core_version_requirement and drupal/core both read ^10 || ^11, and nothing runs on 10.x. All three consuming projects (drupal-base, htdvere, proficio) are on core 11.4.7, and drupal-base is the skeleton every new project starts from, so 10.x had no user and no route to one.

    The floor was not free. .claude/rules/drupal/drupal-modules.md already names #[Hook] attribute classes in src/Hook/ the default for this stack, and this module was the exception to our own doctrine because 10.x has no such registration. FeatureFlags is the concrete case: a static utility whose docblock had to explain that the reason was the supported core range, not a design preference.

    Consumers move by constraint, not by code: ^2.x → ^3.0, no template or call-site edits. Dropping a supported core version is breaking by definition even when nobody feels it, so this releases as 3.0.0.

    Two things the new floor makes dead, removed here because they are dead, not because they are refactors. InterfaceTranslationsKernelTest carried a version fork — the 11.4 locale services when present, locale.translation.inc when not — and the fallback half loaded a file core deprecated in 11.4. And the FeatureFlags docblock stated a constraint that no longer exists.

    Migrating the eight procedural hooks to #[Hook] classes and turning FeatureFlags into an injected service are not in this change. Both become possible here; neither should ride a dependency bump. They land as their own PRs, with their own tests, before 3.0.0 ships.

    drupal/core-dev is pinned to ^11.4 too, so CI resolves against the floor this package actually promises rather than the oldest 11.x composer will accept.

Pull Requests

  • #134 — feat(core)!: require Drupal ^11.4, drop 10.x
  • #135 — refactor(hooks): move every hook to a #[Hook] class
  • #138 — feat(requirements)!: hook_runtime_requirements and the RequirementSeverity enum
  • #140 — refactor(flags)!: make FeatureFlags an injected service
  • #144 — fix(menu): support the renamed service from patch revision #255
  • #145 — fix(file-download)!: return -1 instead of sending a redirect
  • #146 — ci(deprecations): clear the remaining ones and fail on our own

Full Changelog: v2.5.0...v3.0.0