I think also we should add attribute support, as autowiring works elsewhere with attributes users will expect it to be the same here.
Added a question about private Vs protected, I think we perhaps have a valid case for a private property here.
> Every even-numbered .4 release is designated as an LTS.
Where does it say this? On the contrary it appears they call every .4 release an LTS?
And this still makes little difference to Drupal:
Drupal 10 will be EOL in December 2026. Symfony 6.4 will be EOL in November 2027.
We haven't announced an exact date yet but Drupal 11 is likely to be EOL in December 2028. Symfony 7.4 will be EOL in November 2029.
Drupal 12 dates haven't been confirmed, but neither has an end date been set for Symfony 8.
Even if we extended for another eleven months to meet Symfony, we also have to consider our other dependencies such as CKEditor, who won't necessarily make the same promises.
This is all information that hasn't changed since the last issue where you asked for the EOL to be extended, so I'm not sure of the point of rehashing it all again here; you will get the same answer.
Before we could do this we would need confirmation from Symfony of which versions will be LTS, also many years in advance; can you link to any Symfony policy statements where this has been announced? If not, I don't see how we can promise anything up front.
Wondering if we can mark them as deprecated in user.routing.yml then alter the routes in rest.module (if it's installed) to remove the deprecation again. Then in Drupal 12 we move them directly to rest.routing.yml.
When do the deprecations get triggered for deprecated routes? It's not clear to me how people will discover this change if they are using the endpoints externally.
Do we need to change the paths here? It would be better if users of the endpoints didn't have to update their code.
@catch I think 📌 Move JSON:API validation to a feature flag or separate module Active is a good enough followup already?
@andypost see #3552765-7: Update Composer dependencies for 11.3.0 → for reasons why masterminds/html5 breaks tests, probably a duplicate of that issue?
Yes, to put it in simple terms: turn the deprecations into assertions.
CKEditor5 image test failures were due to https://github.com/Masterminds/html5-php/pull/258 - the HTML has two width attributes, and the rule in HTML5 is to take the first value and ignore the latter; this was incorrect in masterminds/html5 until 2.10.
Honestly I think we should just drop that library without deprecation or replacement, it's a small amount of CSS that is very specific to this module and I don't see why we need to go through the deprecation dance for it.
The only major change is doctrine/lexer has gone from 2 to 3. We could still pin at 2, but we added compatibility for 3 back in
📌
[PP-1] Make doctrine/lexer:^3.0 compatible with \Drupal\Component\Annotation\Doctrine.
Active
so probably it's time to upgrade. Otherwise the usual round of minor and patch version bumps.
Two things we can't upgrade to yet are:
- PHPUnit 12 (many issues still in progress)
- PHP_CodeSniffer 4 (dependent on Coder: 📌 Update php_codesniffer to 4.0 Needs work )
+----------------------------------+---------+---------+
| Production Changes | From | To |
+----------------------------------+---------+---------+
| composer/semver | 3.4.3 | 3.4.4 |
| doctrine/deprecations | 1.1.5 | REMOVED |
| doctrine/lexer | 2.1.1 | 3.0.1 |
| masterminds/html5 | 2.9.0 | 2.10.0 |
| pear/archive_tar | 1.5.0 | 1.6.0 |
| symfony/console | v7.3.0 | v7.3.4 |
| symfony/dependency-injection | v7.3.0 | v7.3.4 |
| symfony/error-handler | v7.3.0 | v7.3.4 |
| symfony/event-dispatcher | v7.3.0 | v7.3.3 |
| symfony/filesystem | v7.3.0 | v7.3.2 |
| symfony/finder | v7.3.0 | v7.3.2 |
| symfony/http-foundation | v7.3.0 | v7.3.4 |
| symfony/http-kernel | v7.3.0 | v7.3.4 |
| symfony/mailer | v7.3.0 | v7.3.4 |
| symfony/mime | v7.3.0 | v7.3.4 |
| symfony/polyfill-ctype | v1.32.0 | v1.33.0 |
| symfony/polyfill-iconv | v1.32.0 | v1.33.0 |
| symfony/polyfill-intl-grapheme | v1.32.0 | v1.33.0 |
| symfony/polyfill-intl-idn | v1.32.0 | v1.33.0 |
| symfony/polyfill-intl-normalizer | v1.32.0 | v1.33.0 |
| symfony/polyfill-mbstring | v1.32.0 | v1.33.0 |
| symfony/polyfill-php84 | v1.32.0 | v1.33.0 |
| symfony/polyfill-php85 | v1.32.0 | v1.33.0 |
| symfony/process | v7.3.0 | v7.3.4 |
| symfony/routing | v7.3.0 | v7.3.4 |
| symfony/serializer | v7.3.0 | v7.3.4 |
| symfony/string | v7.3.0 | v7.3.4 |
| symfony/validator | v7.3.0 | v7.3.4 |
| symfony/var-dumper | v7.3.0 | v7.3.4 |
| symfony/var-exporter | v7.3.0 | v7.3.4 |
| symfony/yaml | v7.3.0 | v7.3.3 |
+----------------------------------+---------+---------+
+------------------------------------------------+---------+---------+
| Dev Changes | From | To |
+------------------------------------------------+---------+---------+
| brick/math | 0.12.3 | 0.14.0 |
| composer/ca-bundle | 1.5.6 | 1.5.8 |
| composer/class-map-generator | 1.6.1 | 1.6.2 |
| composer/composer | 2.8.9 | 2.8.12 |
| composer/spdx-licenses | 1.5.8 | 1.5.9 |
| dealerdirect/phpcodesniffer-composer-installer | v1.0.0 | v1.1.2 |
| google/protobuf | v4.31.1 | v4.33.0 |
| justinrainbow/json-schema | 6.5.2 | 6.6.0 |
| micheh/phpcs-gitlab | 2.0.0 | 2.1.0 |
| open-telemetry/api | 1.3.0 | 1.7.0 |
| open-telemetry/context | 1.2.1 | 1.4.0 |
| open-telemetry/exporter-otlp | 1.3.1 | 1.3.2 |
| open-telemetry/gen-otlp-protobuf | 1.5.0 | 1.8.0 |
| open-telemetry/sdk | 1.5.0 | 1.9.0 |
| open-telemetry/sem-conv | 1.32.0 | 1.37.0 |
| phpspec/prophecy-phpunit | v2.3.0 | v2.4.0 |
| phpstan/phpdoc-parser | 2.1.0 | 2.3.0 |
| phpstan/phpstan | 2.1.26 | 2.1.31 |
| ramsey/uuid | 4.8.1 | 4.9.1 |
| sirbrillig/phpcs-variable-analysis | v2.12.0 | v2.13.0 |
| slevomat/coding-standard | 8.18.1 | 8.22.1 |
| squizlabs/php_codesniffer | 3.13.0 | 3.13.4 |
| symfony/browser-kit | v7.3.0 | v7.3.2 |
| symfony/dom-crawler | v7.3.0 | v7.3.3 |
| symfony/lock | v7.3.0 | v7.3.4 |
| tbachert/spi | v1.0.3 | v1.0.5 |
| doctrine/deprecations | NEW | 1.1.5 |
+------------------------------------------------+---------+---------+
longwave → made their first commit to this issue’s fork.
Is there a use case for using the REST login route without having the REST module installed? It seems like a niche thing to do, we could just document in the release notes that you have to enable the REST module now if you are using this.
Proof of concept implementation in MR!13509 - SystemController::themesPage() has two dependencies injected directly.
longwave → made their first commit to this issue’s fork.
To me this is works as designed at this stage. Config now generally lives outside the document root, private files should also for the most secure setup. Changing this seems like a lot of hassle for no real benefit.
Added a BC layer for AnnotationException, and updated the change record.
I am not sure we should provide BC for anything else here. If you were depending on doctrine/annotations, and you required it in composer.json: your code will still work. If you were depending on doctrine/annotations as a transitive dependency of core, I am not sure that is core's responsibility to fix.
Maybe we need a separate map object where we store the arguments once against the class name. Which when I write it out, sounds quite like a container!
Can't reproduce those three fails locally, all those tests pass for me.
Implemented reflection at discovery time in https://git.drupalcode.org/project/drupal/-/merge_requests/13493/diffs over in 📌 Allow plugin service wiring via constructor parameter attributes Needs work
Needs tests, but MR!13493 should improve performance by handling reflection at discovery time for plugins autowired via PluginBase::create().
Retitling this as I think we can narrow the scope a bit.
Looking at the patch this is just a minor hardening for the case where the settings array isn't the expected shape, given this prevents an edge case failure in the upgrade path I think we should just go ahead and commit this.
I think they can be done independently? What I'm kinda proposing here is splitting up the reflection and instantiation parts so they are done at different times, but as long as both parts understand about closures, it doesn't matter whether the reflection is done at discovery or runtime.
📌 Allow plugin service wiring via constructor parameter attributes Needs work landed, which implements this at runtime instead of discovery time. I think we can continue here with adding the reflection step at discovery time and using that data if it's available instead of needing reflection at runtime.
Now we've done this I think we can also simplify 📌 Allow plugin service wiring via constructor parameter attributes Needs work to carry out the reflection at discovery time and store the autowiring configuration in the plugin definition, then we won't need reflection at runtime.
@nicxvan it's only used if the plugin explicitly implements ContainerFactoryPluginInterface, otherwise the factory method is never called, if that helps.
Thanks! Opened 📌 Remove manual create() method from plugins Active as a followup so we can remove a bunch of code from core.
If we're implementing this as best practices only that is fine - but we should add a test to ensure any new info.yml added to core also conforms to the same format.
📌 Keep all sniffs in phpcs.xml.dist in sync with the list got from the locked version of coder Needs work would fix this.
Not sure this really needed a change record, but I'll publish it seeing as there is one.
Committed 3e9f284 and pushed to 11.x. Thanks!
I don't think we need the followup tag any more.
If we are going to enforce this however, can we add a unit test that ensures all core .info.yml files have a prefix?
Change record links are invalid.
Also not very happy about the phpcs:ignore tags as they make the docs super hard to read - more meta tags than useful information! - but not sure what we can do about that.
Backported down to 10.5.x as an eligible bug fix.
Committed and pushed a7b619c813e to 11.x and acd99cccc62 to 11.2.x and cb93ef7747a to 10.6.x and ad75505b110 to 10.5.x. Thanks!
Kinda agree with @alexpott here, it's odd to be triggering any of these as deprecations? Given 📌 Expose API to sort arbitrary config arrays Needs work has been open for four years already I'm wary of adding something "temporary" to the deprecation skips which will likely not turn out to be not that temporary.
+ if ($expected_validated_value !== $value_to_set) {
+ @trigger_error(sprintf('The `type: %s` config schema type uses magical casting when validating.', $key), E_USER_DEPRECATED);
+ }
This is apparently not triggered, so should this be a failure?
For the other cases, shouldn't we assert that we know the cases are bad, then when we fix the cases in followups these tests will start failing, and we will be forced to update them? As we are skipping deprecations this test will get out of date if we are simply ignoring the things it reports instead of making explicit assertions.
Thanks everyone for the discussion, agree that an improved comment is the best we can do here without a full rework.
Backported down to 10.5.x as a docs-only fix.
Committed and pushed 027156e5d1f to 11.x and 3e757eaca9c to 11.2.x and 1d1593ff63c to 10.6.x and 7abe563e1de to 10.5.x. Thanks!
Makes sense to me too, didn't realise you could do this with PHPStan but it will certainly help at higher levels.
Backported down to 10.5.x as a docs-only fix.
Committed and pushed be6b43929c7 to 11.x and 1f4bb78c9e8 to 11.2.x and 661fea386bd to 10.6.x and 14eba7b7a34 to 10.5.x. Thanks!
I don't have a problem with this, we are usually wary of using private anyway in Drupal because we are quite permissive about what we allow downstream users to do, and this seems to be no exception - especially as this is tests only, if someone really wants to override this behaviour in a test, what are we to stop them?
As a feature this is only eligible for minor releases, but I also see no harm in backporting this to 10.6.x.
Committed and pushed bb6ce4e4d46 to 11.x and 27ec7850b62 to 10.6.x. Thanks!
Thanks! Backported to 11.2.x as an eligible bug fix. This could also go back to 10.5.x and 10.6.x but given the use of OOP hooks in the test this will need reworking, marking as fixed for now but if someone wants to open a backport branch then please feel free.
Committed and pushed 75628d32c8a to 11.x and d163dd95b38 to 11.2.x. Thanks!
Published the change record node (but not the change record itself)
Overall I am +1 on doing this, and it will be a long road to getting it done everywhere in core, but added some questions to the MR.
Let's try to add some backward compatibility via moved_classes and/or class_alias().
Thanks for keeping up with this - let's land this now to avoid yet another rebase.
Committed 46c6a19 and pushed to 11.x. Thanks!
I get the same output in 10.6.x with
yarn
yarn vendor-update
yarn build:ckeditor5
yarn build:ckeditor5-types
Copied all the classes that we use into core; DocParserTest still passes locally, let's see if anything else is broken. Unsure if we should copy any other tests from Doctrine.
We forked parts of Doctrine already by copying them into core namespaces, so I think the simplest solution is to do that for the remaining classes that we use.
Can we just reuse opcache.save_comments as the setting? If a site owner chooses to disable that, we skip processing annotations.
I suspect we can remove the jsdoc script and helper file entirely, will open a followup to discuss that.
I think I figured out why PHPStorm works with some files and not others; it's picking up the TypeScript definitions instead of the jsdoc, but CKEditor doesn't have @module tags in all the TS definitions.
command.d.ts has
/**
* @module core/command
*/
...
export declare class Command extends /* #__PURE__ */ Command_base {
and this makes @extends module:core/command~Command work. But editor.d.ts doesn't have a @module tag so it doesn't get detected.
Rebased against 11.x, the performance test changes were unmergeable and will need redoing, otherwise the only big change was the conversion of system_page_top() to OOP.
Fixed up the jsdoc script to handle multiple exports per file. The second regex wasn't matching anything any more so I have removed that one.
Oops, didn't spot those jQuery UI changes, I remember this happening before but I don't remember why, the only change is that ,"ignoreList":[] is added/removed from the end of the map. Reverted this.
Re #14 I think the missing parts are a new bug in the jsdoc script. Previously it only looked for a single default export per file, now there can be multiple exports but I think we only pick up the first. Will see if I can improve this.
I think I fixed the jsdoc parser so it outputs the correct information for CKEditor5 v46 and above, still not sure if this is needed but I think it's as correct now as it was in the previous version.
I've also updated some of the existing jsdoc references for the renames that have occurred in CKEditor.
I think PHPStorm is now picking up the TypeScript types from CKEditor which is why I don't see much difference locally either way, but then e.g. this line still doesn't work locally and I don't understand why:
@extends module:core/plugin~Plugin
Still, that's probably out of scope here now given the jsdoc file appears to be largely correct again.
Firefox still doesn't have support for multiple import maps, but the end of the year is getting closer and we will need this for any CKEditor release in 2026 for 📌 Use New installation methods for CKEditor5 Active
Given that most other browsers do support multiple import maps I think we should go ahead and use the polyfill here.
The changes aren't too bad, just if you are importing code from the API you might need to make some minor renames as per the linked document. The much bigger change comes in 📌 Use New installation methods for CKEditor5 Active which we will have to adapt to soon, because they are stopping support of the DLL loading method at the end of the year.
Some of the jsdoc types need updating as per https://ckeditor.com/docs/ckeditor5/latest/updating/nim-migration/migrat... - this is quite tedious and perhaps can be deferred to a followup. As an example
@param {module:engine/model/element~Element} modelElement
becomes
@param {module:engine/model/element~ModelElement} modelElement
I'm still not clear about #8, because PHPStorm appears to discover some types whether or not the types whether ckeditor5.types.jsdoc exists or not.
I don't really understand the jsdoc build step, but in PHPStorm I can get contextual documentation about any of the imports in the custom plugins without the jsdoc file anyway. I think the break is because of https://ckeditor.com/docs/ckeditor5/latest/updating/nim-migration/migrat... - but perhaps we just don't need this any more?
Actually DLL builds still work, we just need to make some plugin updates I think. A few exports have been changed and injectCssTransitionDisabler() is now CssTransitionDisablerMixin which I couldn't find an upgrade guide for, but seems to work.
The big problem here is that DLL builds are no longer supported and we have to migrate to using JS modules instead, so this is blocked on 📌 Use New installation methods for CKEditor5 Active
yarn build:ckeditor5-types deletes core/modules/ckeditor5/js/build/ckeditor5.types.jsdoc, with the following output:
[09:19:08] CKEditor 5 types have been generated: 0 declarations aliased, 1041 files ignored
Need to figure out what has broken here.
We didn't do this as quickly as I hoped, and v47 is now out; let's try to upgrade to that.
Committed and pushed e4a89e5ef80 to 10.6.x and f1f81bba162 to 10.5.x. Thanks!
We have EditorialContentEntityBase and the generic revision UI now, not sure what else we need to do here.
Things have moved on a lot, with the current work of moving the hooks to OOP and issues like 🐛 HookCollectorPass registers event listeners but they do not follow the required calling convention Active I think this can be closed as outdated.
Only one person requested this, over eight years ago. This is a niche feature that I don't think we need to support in core given the lack of interest, therefore closing this one out.
Thanks for the swift turnaround, let's try this again.
Committed e2aea6e and pushed to 11.x. Thanks!
I am still in favour of doing this as per #39 and #45.
#11 is 5 years old, referencing a blog post from 8 years ago; things have moved on since then.
This is a nice cleanup. Doesn't backport cleanly to 10.x, not sure it is worth the additional effort.
Committed f7bf351 and pushed to 11.x. Thanks!
Given this is a slight behaviour change as discussed in #17 through #26 I think this is best to commit to a minor only.
Committed and pushed 386cfa84bdb to 11.x. Thanks!
Looks good to me too. As it contains a database update this is only eligible for 11.x.
Committed b500857 and pushed to 11.x. Thanks!
Fixed up the docs, removed database drivers for now, and I wrote a basic change record at https://www.drupal.org/node/3549907 →
Yep the screenshot in #3 is saying you can't upgrade to symfony/console 7.3.0 because chi-teck/drupal-code-generator isn't compatible - this is a dependency of Drush, you can either remove Drush or try to upgrade it at the same time.
Composer is telling you that something else requires symfony/console 7.3.0. Try composer why-not symfony/console 7.3.0 which might give you additional clues as to what that is, or if you post your composer.json someone might be able to figure it out.
Fixed up a couple of comments and added types to ::theme().
Removed ThemeEngineInterface::getExtension(), the theme engine is directly responsible for appending file extensions now.
Otherwise, are we done here? Or is there more preprocess to deprecate somewhere?
I think this is just a duplicate of 🐛 RowPluginBase::render() update docblock. Needs work
Tagging as a highlight, improving performance is always good.
This is getting super tricky to deprecate extension.list.theme_engine while still actually needing to use it for various tests, maybe have to revert this and leave it as an unused service that we can just quietly remove in Drupal 13.
Yeah it's not clear in many cases if we should be resetting one, some, or all. Not sure if we need to mess with any of that here though, if there are no known bugs; in practice the lists do not change often.
Following #81 the prefix property on theme value objects is used in a couple of tests, and more importantly ThemeSettingsForm, where we have another magic function name:
if ($theme) {
// Call engine-specific settings.
$function = $themes[$theme]->prefix . '_engine_settings';
if (function_exists($function)) {
I think we can just drop this with no replacement? I had no idea this existed, and if there are custom theme engines somewhere in the wild then they can alter this form in their new custom module instead now.
Unsure if or how to provide BC in the constructor here, but I also don't see why anyone would override this.