Problem/Motivation

entity_browser is not yet Drupal 12 ready. It currently blocks Drupal 12 compatibility for dependent modules — including dropzonejs (#3576977) and paragraphs — because its procedural hook_views_data_alter() in entity_browser.views.inc fatals on Drupal 12 with Class "entity_browser_views_data_alter" does not exist (core no longer auto-discovers procedural hooks from .inc files).

The Drupal 12 compatibility bot did not open an issue automatically, but a bot run produced a rector patch that we can start from: https://git.drupalcode.org/project/project_analysis/-/blob/results-next/results/phpstan-results/entity_browser/entity_browser.2.x-dev.rector.patch

Proposed resolution

Start from the bot rector patch (applies cleanly to 8.x-2.x) and mirror the approach used in dropzonejs #3576977:

1. Apply the bot rector patch — migrates procedural hooks to #[Hook] classes (src/Hook/EntityBrowserHooks.php, src/Hook/EntityBrowserViewsHooks.php, entity_form submodule) with #[LegacyHook] wrappers and services.yml autowire. This moves hook_views_data_alter() into EntityBrowserViewsHooks::viewsDataAlter() and fixes the fatal.
2. Additional fix not in the bot patch: EntityBrowserHooks::formAlter() used $request->get('_route');. Request::get() was removed in Symfony 7 (D12) — replaced with \Drupal::routeMatch()->getRouteName() .
3. core_version_requirement => ^10.2 || ^11 || ^12.
4. .gitlab-ci.yml: enable next-minor / next-major testing and add the not-yet-D12-ready test cross-dependencies (paragraphs, entity_reference_revisions, entity_embed, embed, entityqueue, inline_entity_form, media_entity, search_api, token) to _LENIENT_ALLOW_LIST so composer can resolve on the next major and we get real test results.
5. Remaining PHP 8.5 / Drupal 12 deprecations (native return type declarations on WidgetValidationBase and peers, null array offsets in plugin collections) — a further rector sweep.

Remaining tasks

- Full D12 deprecation sweep (step 5).
- Get functional/js tests green — will require at least one cross-dependency (e.g. paragraphs) to be merged without fully green results, as these projects have circular/cross dependencies (per @berdir in #3576977).

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

velmir_taky created an issue. See original summary.

velmir_taky’s picture

Issue summary: View changes

velmir_taky’s picture

Assigned: velmir_taky » Unassigned
Issue summary: View changes
Status: Active » Needs review
Related issues: +#3576977: Fix Drupal 12 deprecations: replace FormElement, renderPlain() and FileSystemInterface::EXISTS_RENAME

Hi @berdir, thanks a lot for the pointers on #3576977 — this got the ball rolling nicely.
Pushed the first pass (MR !150), here's a friendly summary of where things stand.

What this MR does

Starting from bbrala's bot rector patch, plus mirroring what we did in dropzonejs #3576977:

- Procedural hooks => OOP #[Hook] classes (EntityBrowserHooks, EntityBrowserViewsHooks, and the entity_form submodule) with #[LegacyHook] wrappers and services.yml autowire. This moves hook_views_data_alter() into EntityBrowserViewsHooks::viewsDataAlter() and clears the entity_browser_views_data_alter fatal.

- Split the hooks with proper DI, so there are no \Drupal:: calls left in the new hook classes (phpstan-friendly), just like you suggested for dropzonejs.

- Three extra install-blocking D12 fatals the bot patch didn't cover, caught by running the suite against real 12.0-dev:
1. EntityBrowserFormController::getFormObject() used $this->request->get() (Request::get() is gone in Symfony 7) => ->query->get('original_path').

2. EntityBrowserListBuilder::getDefaultOperations() — D12 added a second CacheableMetadata $cacheability argument. Made the override compatible across 10.2–12 (?CacheableMetadata $cacheability = NULL + null-safe cacheability), and added a small regression kernel test (EntityBrowserListBuilderTest) so this can't sneak back.

- core_version_requirement => ^10.2 || ^11 || ^12, .gitlab-ci.yml next-minor/next-major + _LENIENT_ALLOW_LIST, and #[Group] / #[RunTestsInSeparateProcesses] for PHPUnit 12.

CI picture (and the good news)

- Current core (11.4.4): 53/57 green. The 4 red ones are all entity_embed + CKEditor5 (CardinalityTest::testEntityEmbed and the three EntityEmbedTest cases), failing inside core's CKEditor5TestTrait. These are not from this MR — the base 8.x-2.x pipeline on the same commit (54253b3, pipeline #896776) fails the exact same four, and they're already being handled in #3612718. The new EntityBrowserListBuilderTest passes on both 11.4 and 12.0-dev.

- Next major (D12): the remaining failures are all Class "Drupal\Core\Render\Element\FormElement" not foundFormElement was removed in D12 and EntityBrowserElement / EntityBrowserPagerElement still extend it. That's the one big blocker left, and it's already tracked in #3565236 (fix is FormElement => FormElementBase, which needs 10.3).

============================================

Would you rather I fold the FormElement => FormElementBase change into this MR (bumping to ^10.3 and dropping 10.2, as we did in dropzonejs) so next-major goes much greener — or keep that in #3565236 and keep this MR focused on the hook/OOP + fatal fixes? Happy either way, just don't want to step on that issue's scope.

The remaining phpcs/phpstan/eslint/cspell/stylelint reports are the module's existing baseline (the new hook classes are clean); those look like they belong in #3541968 rather than here.

Thanks again!

berdir’s picture

Status: Needs review » Needs work

Lets merge in the FormElementBase changes into this issue, because both issues will need to change .info.yml and then they'll conflict. We can merge contribution credits.

Also reviewed the MR.

velmir_taky’s picture

Thanks @berdir, all addressed and pushed.

A couple of things worth calling out beyond the diff:
- trustData() — since it's deprecated in 11.4 anyway, I just dropped it rather than swapping the call, per your hint.

- views.inc — I moved the leftover #[LegacyHook] wrapper into .module and deleted the .inc (it was still tripping the "autoloading hooks in .inc is removed in D12" deprecation). Shout if you'd rather keep that for the follow-up.

- Folded in FormElementBase + ^10.3 as discussed — credits merge sounds good.

On CI: the current-core reds are just the entity_embed/CKEditor5 ones from #3612718 (same on plain 8.x-2.x). Next-major is green for everything this MR touches; what's left there is those CKEditor tests, a few not-yet-D12 cross-deps from the lenient list, and modal.js choking on jQuery 4 — that last one's a separate D12 JS follow-up, no JS changed here.

berdir’s picture

Status: Needs review » Needs work

> and modal.js choking on jQuery 4 — that last one's a separate D12 JS follow-up, no JS changed here.

I'm hesitant to mark a module as D12 compatible when there are known issues. I know little about JS, so that's trickier for me to review. Often these changes are pretty small in scope, so I think we should either include it, or consider this a preparation step for D12 without explicitly marking it as compatible (so remove the .info.yml change for D12 but keep 10.3). That would be enough to unblock dropzonejs/paragraphs testing.

berdir’s picture

I merged the test fixes, so phpunit job should be green now after rebasing.

One big thing that we definitely should push to a follow-up is attribute support for plugins. Core still hasn't decided on whether it should enforce the D12 deprecation around that (that plugin managers need to support attributes), but it's better to be prepared for that. There are several plugin managers, so we need to provide an attribute class for all of them and also convert all our own plugins and the ones from core.

Since we don't know yet whether or not core will require attribute classes, I think it's clear now to me to not yet flag the module as D12 compatible here. Instead, lets create a parent issue "Drupal 12 compatibility" with this issue as child + two new follow-ups, for the JS and one for the plugins.

Appreciate any help but also no expectations, I'll eventually get to it myself.

velmir_taky’s picture

velmir_taky’s picture

Status: Needs work » Needs review

Sounds like a good plan, and agreed on not flagging D12 compatible until the JS and plugin-attribute pieces are sorted — the uncertainty around core enforcing attribute classes is a good reason to hold.

Set up the tree as you suggested:
- Parent: #3613379: Drupal 12 compatibility
- This issue (#3613186) — moved under it as a child. I removed the || ^12 from entity_browser.info.yml (kept ^10.3 for FormElementBase), so it's now purely the D12 preparation step — OOP hooks, the install-blocking fatals, FormElementBase — without claiming compatibility. Still enough to unblock dropzonejs/paragraphs testing via the lenient list. Rebased on the merged test fixes so the phpunit job is green.
- JS follow-up: #3613381: Fix jQuery 4 (Drupal 12) deprecations in JavaScript — child of the parent, MR up. It's a mechanical sweep, verified to be a no-op on the current jQuery 3.x, so no behaviour change on 11.x; the next-major CI job is where it earns its keep (that's where modal.js was throwing under jQuery 4).

For the plugins piece: turns out there's already #3522955: Use PHP attributes instead of doctrine annotations for plugin type definitions — and you'd already expanded its scope in #10 there to also cover the core field/constraint/views plugin conversions. Rather than open a duplicate I've linked it as the third child of #3613379. Shout if you'd rather have a fresh one scoped specifically to "attribute classes for every plugin manager".

berdir’s picture

Status: Needs review » Postponed
berdir’s picture

Status: Postponed » Needs review

In Hindsight, it might have been better to do this first as I added a hook over there, but it would have conflicted either way. But the conflicts weren't too bad, rebased and finalizing now.

  • berdir committed 00b01502 on 8.x-2.x authored by velmir_taky
    task: #3613186 Fix Drupal 12 deprecations: migrate hooks to OOP classes...
berdir’s picture

Status: Needs review » Fixed

seems good, current and previous are green, next minor is just failing on that deprecation stuff. merged.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.