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
Issue fork entity_browser-3613186
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
Comment #2
velmir_taky commentedComment #4
velmir_taky commentedHi @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 theentity_formsubmodule) with#[LegacyHook]wrappers andservices.ymlautowire. This moveshook_views_data_alter()intoEntityBrowserViewsHooks::viewsDataAlter()and clears theentity_browser_views_data_alterfatal.- 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 secondCacheableMetadata $cacheabilityargument. 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.ymlnext-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::testEntityEmbedand the threeEntityEmbedTestcases), failing inside core'sCKEditor5TestTrait. These are not from this MR — the base8.x-2.xpipeline on the same commit (54253b3, pipeline #896776) fails the exact same four, and they're already being handled in #3612718. The newEntityBrowserListBuilderTestpasses on both 11.4 and 12.0-dev.- Next major (D12): the remaining failures are all
Class "Drupal\Core\Render\Element\FormElement" not found—FormElementwas removed in D12 andEntityBrowserElement/EntityBrowserPagerElementstill extend it. That's the one big blocker left, and it's already tracked in #3565236 (fix isFormElement=>FormElementBase, which needs 10.3).============================================
Would you rather I fold the
FormElement=>FormElementBasechange into this MR (bumping to^10.3and 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!
Comment #5
berdirLets 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.
Comment #6
velmir_taky commentedThanks @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.moduleand 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.3as 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.jschoking on jQuery 4 — that last one's a separate D12 JS follow-up, no JS changed here.Comment #7
velmir_taky commentedComment #8
berdir> 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.
Comment #9
berdirI 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.
Comment #10
velmir_taky commentedComment #11
velmir_taky commentedSounds 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
|| ^12fromentity_browser.info.yml(kept^10.3forFormElementBase), 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.jswas 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".
Comment #12
berdirI'm finishing up #3522955: Use PHP attributes instead of doctrine annotations for plugin type definitions first, that will conflict a bit anyway.
Comment #13
berdirIn 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.
Comment #15
berdirseems good, current and previous are green, next minor is just failing on that deprecation stuff. merged.