Fix Drupal 12 deprecations: replace FormElement, renderPlain() and FileSystemInterface::EXISTS_RENAME

Thanks @titacvetkovic for raising this and the initial patch!

I took a closer look and found a few things that needed adjusting, so I pushed a corrected MR:

- The @FormElement("dropzonejs") annotation must stay as-is — it's used for plugin discovery, only the parent class changes to FormElementBase
- renderPlain() replacement should be renderInIsolation(), not render() — they have different behavior (see #3407994: RendererInterface::renderPlain() is deprecated in favour of ::renderInIsolation())
- Removed unused use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface import (interface no longer exists in current Symfony)

All existing kernel tests pass.

These are deprecations in Drupal 11 for Drupal 12.

I think they require 10.3, so we need to update .info.yml and drop D9 and require 10.3

At least in the form element, we could also change to attributes instead of annotations if we change that anyway.

Created MR !31 for the hook class migration and core_version_requirement update, which addresses the feedback from comment #6 and #7:
https://git.drupalcode.org/project/dropzonejs/-/merge_requests/31

This MR:
- Adds `src/Hook/DropzoneJsHooks.php` with all hook implementations moved to a class using `#[Hook]` attributes
- Updates `dropzonejs.module` to delegate from procedural `#[LegacyHook]` wrappers to the new class
- Registers the class in `dropzonejs.services.yml` with `autowire: true`
- Updates `core_version_requirement` to `^10.3 || ^11 || ^12` in both info.yml files (drops D9 support, adds D12)

Note: MR !29 handles the deprecated API replacements (FormElement, renderPlain, FileSystemInterface::EXISTS_RENAME) and should be merged first, or this MR should be rebased on top of it. Requesting review from other maintainers.

Done per #6 and #7: bumped core_version_requirement to ^10.3 || ^11 || ^12 in both info.yml files (drops D9, matches !31), and converted the render element from the @FormElement annotation to the #[FormElement] attribute. The deprecated-API replacements (FormElementBase, renderInIsolation, FileExists::Rename) are unchanged.

Rebased on the latest 8.x-2.x so it fast-forwards, and aligned the core constraint with !31 so there's no conflict when that one rebases on top. The hook migration stays in !31 as @wouters_f planned in #8.

Back to Needs review.

There's also the bot issue: #3609930: Automated Drupal 12 compatibility fixes for dropzonejs 2.x-dev, might make sense to check that for anything that was missed here, merge it together an then close the bot issue.

Then, start changing .gitlab-ci.yml to enable next minor and major testing to get actual test results. Very likely projects like entity_browser will need to be updated first to get this to green on D12.

Done per #10:
- Enabled next-minor + next-major (D12) testing in .gitlab-ci.yml.
- Merged the bot fixes from #3609930: Automated Drupal 12 compatibility fixes for dropzonejs 2.x-dev: pulled in the UploadController::handleUploads() change (native basename(), dropped the now-unused $file_system). The hook migration + services.yml autowire from that bot MR are already covered by !31.

Once this + !31 land, #3609930: Automated Drupal 12 compatibility fixes for dropzonejs 2.x-dev can be closed as merged. Next-major will likely be red at composer until entity_browser is D12-ready, as noted in #10.

One thing you could additionally try to see if we can at least some of the test to run and identify some more necessary fixes is to add entity_browser to _LENIENT_ALLOW_LIST like pathauto: https://git.drupalcode.org/project/pathauto/-/blob/8.x-1.x/.gitlab-ci.ym...

And then manually start the next major composer job.

Also, if at all possible, lets next previous major as well to make sure it actually still works on D10. I can't remember right now if there's an existing issue with that, but we can always try.

Reproduced next-major locally against real Drupal 12.0-dev (PHP 8.5, drupalci image) and fixed all module-side failures — verified green on both D12 and D11:

- FormElement/renderPlain/EXISTS_RENAME — the three original deprecations (FormElementBase, renderInIsolation, FileExists::Rename) + @FormElement => #[FormElement] attribute.
- core_version_requirement => ^10.3 || ^11 || ^12 (both info.yml), dropping D9 per #6/#7.
- Bot #3609930 merged (UploadController native basename()); hook migration stays in !31.
- CI: enabled previous-major / next-minor / next-major, plus _LENIENT_ALLOW_LIST: entity_browser.
- Test discovery (PHPUnit 12): #[Group] + #[RunTestsInSeparateProcesses] on all test classes (CR).
- REQUIREMENT_OK/REQUIREMENT_ERROR removed in D12 → RequirementSeverity enum with a class_exists() guard for 10.3 BC (CR).
- template_preprocess_dropzonejs() stopped running on D12 (core removed template_preprocess_HOOK() auto-discovery) => renamed to hook_preprocess_dropzonejs().

Result on 12.0-dev: all 4 kernel-suite classes pass (DropzoneJsEbWidgetTest errors only because of entity_browser's own D12 issue — Class "entity_browser_views_data_alter" does not exist, upstream, not this module).

Was a bit confused with the two merge requests. !31 seems to be the same fork, but doesn't show up in the issue properly. We rarely do multiple merge requests per issue (unless it's different branches or follow-ups), I'd prefer having them merged together so we can test the whole picture.

Will do a proper code review then, but will probably look into getting entity_browser ready first before merging this, it's also blocking paragraphs. Didn't check yet what the status there is.

Thanks @berdir. You're right about the two MRs — !31's branch (d12-hooks-info) was pushed directly on the project repo, which is why it never attached to the issue.

I've folded !31's hook-class migration into !29, so everything now lives in a single MR:

- src/Hook/DropzoneJsHooks.php — all hooks moved to a class with #[Hook] attributes (help, theme, preprocess, library_info_build/alter, media_source_info_alter, form_alter)
- dropzonejs.module — reduced to thin #[LegacyHook] wrappers
- dropzonejs.services.yml — registers the hook class with autowire: true

The OOP #[Hook('preprocess_dropzonejs')] supersedes the earlier template_preprocess_dropzonejs rename, so that's cleaner now too. !31 can be closed as merged here.

Tested against real 12.0-dev (PHP 8.5): the three module-side kernel tests pass (37 assertions, no failures). DropzoneJsEbWidgetTest still only errors on entity_browser's own D12 issue, so that remains the real blocker as you noted — happy to help there if useful.

Two minor non-blockers I spotted while testing, if you'd want them here or as a follow-up:

- dropzonejs_requirements wants a #[LegacyRequirementsHook] attribute (deprecated 11.3, removed 13.0 — CR 3549685). Tricky to do cleanly while keeping ^10.3, so maybe a follow-up.
- UploadController::create() triggers the soft "might add static return type" notice.

> - dropzonejs_requirements wants a #[LegacyRequirementsHook] attribute (deprecated 11.3, removed 13.0 — CR 3549685). Tricky to do cleanly while keeping ^10.3, so maybe a follow-up.

This shouldn't be an issue for D10, it's just an attribute, that alone won't cause any issues.

The tricky part is that it runs on all phases, including install, which is different now. Relying on a service there is tricky, but it works now, and it's not a service of of the module but core, so that should be fine. We might need to duplicate the code for the runtime/update phases (that can be a single method with both Hook attributes) and the Install requirements class. Maybe we can call out to the install requirements class ourself too.

It's only a D13 deprecation, but so are all other hooks and the deprecation is quite noisy.

> - UploadController::create() triggers the soft "might add static return type" notice.

I think the most useful way to handle that is rely on AutowireTrait: https://www.drupal.org/node/3395716, that's available since 10.2. But this is already big, happy to do that in a separate issue.

> DropzoneJsEbWidgetTest still only errors on entity_browser's own D12 issue, so that remains the real blocker as you noted —
> happy to help there if useful.

Help is welcome. I noticed that there's no bot issue yet, something seems to have confused the bot. But @bbrala pointed me to the patch file from the bot run, we can start with that with our own issue: https://git.drupalcode.org/project/project_analysis/-/blob/results-next/...

EB is tricky because paragraphs and possibly others have a cross-dependency on each other and it has a ton of dependencies in its tests, some of which are not actively maintained, we'll need to merge at least one project without having green results to move forward.

But we can start, with similar steps as here, bot patch + .gitlab-ci.yml updates and so on. If you start, add a reference the issue here.

Thanks for the detailed review @berdir — all of your inline comments are addressed. The module code is now green on every core version the CI runs: the kernel suite passes on previous-major (D10), 11.2, next-minor (11.4) and next-major (12.0-dev), and the functional DropzoneJsEbWidgetTest passes on 10/11/11.4. The only red left is next-major, where that functional test errors on entity_browser's own D12 issue (Class "entity_browser_views_data_alter" does not exist) and where the deprecation warnings come from entity_browser (annotation => attribute plugin discovery, entity_browser.views.inc autoloading) plus core — nothing from this module. So this is now effectively blocked only on entity_browser, as you anticipated.

Per your threads:

- formatPlural — now comes from StringTranslationTrait ($this->formatPlural(...)); dropped the \Drupal::translation() call.

- Hook split + DI — theme, preprocess and library moved to a new src/Hook/DropzoneJsThemeHooks.php with LibrariesDirectoryFileFinder injected via the constructor. help, media_source_info_alter and form_*_alter stay in DropzoneJsHooks. Both classes are registered autowire: true in dropzonejs.services.yml, and the legacy #[LegacyHook] wrappers in .module now delegate to the right service.

- initial preprocess — set on the theme hook itself: 'initial preprocess' => self::class . ':preprocessDropzonejs', so it runs first like before, and removed the separate #[Hook('preprocess_dropzonejs')]. One compatibility note: 'initial preprocess' only exists since 11.2.0, but we require ^10.3, so I kept a thin template_preprocess_dropzonejs() in .module that delegates to the same class method — it's the only thing that populates the variables on 10.3–11.1. On 11.2+/12 core skips discovering it (because the 'initial preprocess' callback is set), so it triggers no deprecation. @todo to drop it once we require 11.2.

- @see / legacy references — updated to point at the class method.

- requirements — clean split, and covering all phases per your #16 note that the old hook runs on install/update/runtime: the check lives once in src/Install/Requirements/DropzonejsRequirements.php (InstallRequirementsInterface::getRequirements(), install phase); src/Hook/DropzoneJsRequirements.php has a single method carrying both #[Hook('runtime_requirements')] and #[Hook('update_requirements')] that delegates to it. The legacy dropzonejs_requirements() keeps the REQUIREMENT_* constants and is marked #[LegacyRequirementsHook] (all phases, pre-11.3 only).

- requirements test — uses \Drupal::service('system.manager')->listRequirements() on 11.2+ (so it verifies the OOP hook is actually registered and invoked), and falls back to calling the legacy dropzonejs_requirements() directly on older core, where listRequirements() doesn't pick up the procedural hook in a kernel test — the "duplicate what we call based on the core version" you suggested. This is what makes previous-major (D10) green.

Two things you flagged as separate follow-ups, keeping them out of this MR as you preferred:

- UploadController::create() => AutowireTrait (CR 3395716). I'll open a follow-up issue.
- entity_browser D12-readiness — happy to take this on since it blocks this MR (and paragraphs). I'll open a dedicated issue starting from the bot rector patch bbrala pointed to and mirror the CI steps we did here, and reference it back here.

I've opened the dedicated D12-readiness issue for it and pushed a first MR: #3613186: Fix Drupal 12 deprecations: migrate hooks to OOP classes and fix views_data_alter fatal (entity_browser MR !150). It starts from bbrala's bot rector patch and mirrors the CI/OOP-hook approach we did here — procedural hooks => #[Hook] classes, plus the entity_browser_views_data_alter fatal and a couple of other D12 fatals fixed. entity_browser's own tests are green on 11.4 and the new kernel test passes on 12.0-dev too.

So the blocker for this MR is now actively moving over there. I'll keep referencing progress back here as it lands.

Thanks again for the thorough review!

There's a test fail on some but not all branches about the file extensions, this could be related to an entity_browser issue I merged about not hardcoding the supported extensions...

Don't think this one is #3582928: FileBrowserWidget.php strips extensions other than 'png', 'gif', 'jpg', 'jpeg' - that only touches FileBrowserWidget (file/image fields), and this test goes through an entity_reference => media field. It fails the same way on old and current entity_browser, so it's not the extension change. Looks like a recent core release made the iframe render dynamic-page-cacheable and surfaced a latent bug.

Reproduced with real Chrome: handleWidgetContext() runs only once across the two page loads - the second drupalGet() gets the iframe stale from Dynamic Page Cache, so the config change never shows up. The inherit logic is fine, the form just never declared the cacheability of the config acceptedFiles is built from.

Fixed by making the widget form depend on the Entity Browser config, and on the media source field when settings are inherited. Extended the existing test instead of adding a new one - green on 11.2 and 12.0-dev now.

Nice find. Maybe we should push that up into entity_browser, because every entity browser always depends on its config, any widget has settings that it depends on, including which widgets you have on that. dropzonejs isn't special about that, just manages to trigger it in the test?

Yeah, agreed - nothing dropzonejs-specific here, the whole browser form is built from its config. Pushed the fix up to entity_browser: #3616416: Entity browser form does not declare its config as a cache dependency.

Once it lands I'll drop the generic line here and keep just the media source field part (inherited file_extensions), which is the only bit that's actually ours.