GitLab also says there is a merge conflict?
Discussed this with @larowlan and @quietone. This MR adds a new Mailer module that is alpha stability. As per the experimental modules guidelines → , alpha stability modules may be removed from releases, and in this case the module is not really useful on its own at the present time. Therefore, instead of committing this to 11.x now for 11.2 (only to have to remove it again), we agreed that the best course of action is to commit this to 11.x shortly after 11.2.x has been branched, with the aim to meet beta stability requirements over the next 6 months and include this as experimental in 11.3.0.
Thank you @znerol for your patience and continuing work in this space.
#19 reminded me of 🐛 Enable header-based proactive content negotiation with optimizations and opt-outs available. Needs work where Drupal does not use the Accept header for format negotiation, primarily because of external caching issues.
That makes me think we should just set the query string on the client side, even if we also have the header set.
This is more of a highlight than something that needs a release note.
Looks good to me.
I assume retry_period
from the changelog is not relevant to us?
@andypost Probably not until Drupal 12.
Some bikeshedding in my review comments.
Also discussed this with @nod_ who pointed out that while we added the HTMX library already, it's not really much use without these changes - behaviors and assets are critical to integrating with the rest of Drupal's frontend - so tagging as an 11.2 release priority to allow others to start actually looking at how they might want to use this.
Gonna go ahead and mark this RTBC so we can continue to move the HTMX proposal forward; given that this is currently experimental in core if we have to make any API changes or fixes then we should still be safe. Although it could do with another review from someone who writes more JavaScript than I do, I found the MR easy to follow and everything in it makes sense to me.
We do need the query string, as that's how MainContentViewSubscriber
decides which instance of MainContentRendererInterface
is needed to render the page; without the query string it will fall back to HtmlRenderer
.
It passes tests, but I'm not sure how much we exercise the code any more, given we have few annotations left in core...
For the idea in #6 we would need HTMX to send ?_wrapper_format=htmx
in the query string of all requests. This looks like it is doable via the htmx:configRequest event. Then, we add a service similar to HtmlRenderer
but which can do less work - it wouldn't need to care about display variants at all.
Thanks again - let's try to get this into 11.2 so contrib actually finds out about some of these that may have previously been skipped.
Thanks for reviewing. I split out the PHPUnit one but think we should leave DrupalSelenium2Driver for now.
2 added an object with ArrayAccess support and 3 dropped this support finalizing the move to object.
Given we control \Drupal\Component\Annotation\Doctrine\DocParser
and it's already final, can we make it compatible with the object way of doing things, and allow ^2 || ^3
in composer.json, but stay pinned to ^2
(for now) in core-recommended? This adds forward compatibility for anyone who wants to upgrade for other reasons, but stays conservative otherwise - but also opens the door to finishing the upgrade in Drupal 12 if we want to. If the token arrays/objects are not leaked outside of the implementation this seems like it might be workable.
Notes for reviewers are in the MR.
I was wrong about Select::hasAllTags()
and friends, but we can tighten up many of the other skips and remove a few - two where our implementations have been removed and one where we don't need to use a custom stub in a test.
I think this should be closed as outdated, Automatic Updates → uses PHP-TUF to improve supply chain security, see https://rugged.works/background/tuf_for_humans/ for more information.
Let's ship this before it needs yet another reroll.
Committed f0f2901 and pushed to 11.x. Thanks!
longwave → created an issue.
longwave → created an issue.
New deprecations in Twig 3.21, fixed directly instead of ignoring them.
Rebased, added some more updates that have landed since, the full set is now:
+----------------------------------+---------+--------------+
| Production Changes | From | To |
+----------------------------------+---------+--------------+
| doctrine/deprecations | 1.1.3 | 1.1.5 |
| egulias/email-validator | 4.0.2 | 4.0.4 |
| guzzlehttp/guzzle | 7.9.2 | 7.9.3 |
| guzzlehttp/promises | 2.0.4 | 2.2.0 |
| guzzlehttp/psr7 | 2.7.0 | 2.7.1 |
| mck89/peast | v1.16.3 | v1.17.0 |
| revolt/event-loop | v1.0.6 | v1.0.7 |
| symfony/console | v7.2.0 | v7.3.0-BETA1 |
| symfony/dependency-injection | v7.2.0 | v7.3.0-BETA1 |
| symfony/error-handler | v7.2.0 | v7.2.5 |
| symfony/event-dispatcher | v7.2.0 | v7.3.0-BETA1 |
| symfony/filesystem | v7.2.0 | v7.3.0-BETA1 |
| symfony/finder | v7.2.0 | v7.3.0-BETA1 |
| symfony/http-foundation | v7.2.0 | v7.3.0-BETA1 |
| symfony/http-kernel | v7.2.0 | v7.3.0-BETA1 |
| symfony/mailer | v7.2.0 | v7.3.0-BETA1 |
| symfony/mime | v7.2.0 | v7.3.0-BETA1 |
| symfony/polyfill-ctype | v1.31.0 | v1.32.0 |
| symfony/polyfill-iconv | v1.31.0 | v1.32.0 |
| symfony/polyfill-intl-grapheme | v1.31.0 | v1.32.0 |
| symfony/polyfill-intl-idn | v1.31.0 | v1.32.0 |
| symfony/polyfill-intl-normalizer | v1.31.0 | v1.32.0 |
| symfony/polyfill-mbstring | v1.31.0 | v1.32.0 |
| symfony/process | v7.2.0 | v7.3.0-BETA1 |
| symfony/psr-http-message-bridge | v7.2.0 | v7.3.0-BETA1 |
| symfony/routing | v7.2.0 | v7.3.0-BETA1 |
| symfony/serializer | v7.2.0 | v7.3.0-BETA1 |
| symfony/string | v7.2.0 | v7.2.6 |
| symfony/validator | v7.2.0 | v7.3.0-BETA1 |
| symfony/var-dumper | v7.2.0 | v7.2.6 |
| symfony/var-exporter | v7.2.0 | v7.2.6 |
| symfony/yaml | v7.2.0 | v7.3.0-BETA1 |
| twig/twig | v3.19.0 | v3.21.1 |
+----------------------------------+---------+--------------+
+------------------------------------+----------+---------+
| Dev Changes | From | To |
+------------------------------------+----------+---------+
| brick/math | 0.12.1 | 0.12.3 |
| composer/ca-bundle | 1.5.4 | 1.5.6 |
| composer/class-map-generator | 1.5.0 | 1.6.1 |
| composer/composer | 2.8.3 | 2.8.6 |
| google/protobuf | v4.29.1 | v4.30.2 |
| micheh/phpcs-gitlab | 1.1.0 | 2.0.0 |
| myclabs/deep-copy | 1.12.1 | 1.13.1 |
| nikic/php-parser | v5.3.1 | v5.4.0 |
| open-telemetry/api | 1.1.1 | 1.2.3 |
| open-telemetry/context | 1.1.0 | 1.2.1 |
| open-telemetry/exporter-otlp | 1.1.0 | 1.2.1 |
| open-telemetry/gen-otlp-protobuf | 1.2.1 | 1.5.0 |
| open-telemetry/sdk | 1.1.2 | 1.3.0 |
| open-telemetry/sem-conv | 1.27.1 | 1.32.0 |
| phpdocumentor/reflection-docblock | 5.6.1 | 5.6.2 |
| phpspec/prophecy | v1.20.0 | v1.22.0 |
| phpstan/phpdoc-parser | 1.33.0 | 2.1.0 |
| phpunit/phpunit | 10.5.38 | 10.5.46 |
| ramsey/collection | 2.0.0 | 2.1.1 |
| sirbrillig/phpcs-variable-analysis | v2.11.21 | v2.12.0 |
| slevomat/coding-standard | 8.15.0 | 8.18.0 |
| squizlabs/php_codesniffer | 3.11.1 | 3.12.2 |
| symfony/browser-kit | v7.2.0 | v7.2.4 |
| symfony/dom-crawler | v7.2.0 | v7.2.4 |
| symfony/lock | v7.2.0 | v7.2.6 |
| tbachert/spi | v1.0.2 | v1.0.3 |
+------------------------------------+----------+---------+
Needs reroll.
Duplicate of 📌 Update Composer dependencies for 11.2.0 Active I think?
@andypost see 📌 Update Composer dependencies for 11.2.0 Active for the beta updates, this was intended for the final release - I should have marked it postponed.
Thanks for the update, this should help anyone else in a similar situation.
Added some questions/nits from reading the code only, haven't done any manual testing yet. Leaving at RTBC to encourage other reviewers :)
+1 for the decision in #38, given we have seen prior art in using meta:enum
but that also the x-
prefix is seemingly preferred by the JSON Schema team for extensions.
I think cascading the context makes sense too. It seems likely that you would want the same context to be applied across all translatable strings in the prop (and certainly all cases in the enum), but you might also want it to cover the whole component without having to repeat yourself.
@pdureau are you OK with this?
Committed 9d28549 and pushed to 11.x. Thanks!
I agree this could do with a bit more cleanup but let's defer that to 📌 Rethink #[Hook] attribute inheritance Active .
Looks good to me.
It's somewhat disappointing how difficult these seemingly simple config schema changes are to actually carry out, but I think this one is solid now, and the BC patterns and test coverage we figured out here can hopefully be reused in similar future issues.
Added a question about historical string values.
Some CSS linting issues that need resolving manually, and I don't immediately know how
modules/ckeditor5/css/drupalmedia.css
31:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/components/toolbar-button/toolbar-button.css
39:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/components/toolbar-button/toolbar-button.pcss.css
36:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/css/components/toolbar-menu.css
54:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/css/components/toolbar-menu.pcss.css
58:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/css/components/toolbar-message.css
18:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
modules/navigation/css/components/toolbar-message.pcss.css
10:15 ✖ Unexpected deprecated keyword "break-word" for property "word-break" declaration-property-value-keyword-no-deprecated
themes/claro/css/components/form--text.css
50:19 ✖ Unexpected unknown value "-webkit-baseline-middle" for property "vertical-align" declaration-property-value-no-unknown
themes/claro/css/components/form--text.pcss.css
41:19 ✖ Unexpected unknown value "-webkit-baseline-middle" for property "vertical-align" declaration-property-value-no-unknown
🐛 JS component slots don't appear in the preview canvas until published Active was fixed in 🐛 Hovered preview for JS components in library not working Active
longwave → created an issue.
@nod_ I think we also need to delete assets/vendor/htmx/debug.js
from the repo?
longwave → made their first commit to this issue’s fork.
Needs work for #3 plus test failures.
In the CSS update issue we should also update the browser data:
Browserslist: browsers data (caniuse-lite) is 7 months old. Please run:
npx update-browserslist-db@latest
Why you should do it regularly: https://github.com/browserslist/update-db#readme
yarn build:ckeditor5
results in some warnings
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalelementstyle/drupalelementstyleediting.js 240:16-21
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalelementstyle.js 5:0-87 40:12-37
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 8:0-54 24:2-20
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalelementstyle/drupalelementstyleediting.js 241:27-32
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalelementstyle.js 5:0-87 40:12-37
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 8:0-54 24:2-20
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediacaption/drupalmediacaptionui.js 37:14-27
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediacaption.js 5:0-77 17:39-59
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 9:0-54 23:2-20
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/mediaimagetextalternative/mediaimagetextalternativeui.js 69:14-29
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 12:0-98 21:2-29
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/mediaimagetextalternative/ui/textalternativeformview.js 53:6-17
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/mediaimagetextalternative/mediaimagetextalternativeui.js 18:0-67 102:21-44
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 12:0-98 21:2-29
WARNING in ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/mediaimagetextalternative/ui/textalternativeformview.js 63:6-18
export 'icons' (imported as 'icons') was not found in 'ckeditor5/src/core' (possible exports: Command, Context, ContextPlugin, DataApiMixin, Editor, ElementApiMixin, MultiCommand, PendingActions, Plugin, attachToForm, secureSourceElement)
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/mediaimagetextalternative/mediaimagetextalternativeui.js 18:0-67 102:21-44
@ ./modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/index.js 12:0-98 21:2-29
longwave → created an issue.
This MR updates the following, which should be mostly uncontroversial:
- yarn
- @floating-ui/dom
- backbone
- chokidar
- dotenv
- glob
- jsdom
- nightwatch
- terser
- terser-webpack-plugin
- webpack
- webpack-cli
I decided to leave CKEditor, cspell, eslint and prettier/postcss/stylelint to their own issues because they usually end up with more widespread changes.
Locally when I run yarn vendor-update
on 11.x I get some changes without changing anything in package.json or the lockfile:
assets/vendor/htmx.org
is created, which is a copy ofassets/vendor/htmx
htmx.debug
is upgraded from 2.0.1 to 2.0.4
longwave → made their first commit to this issue’s fork.
It seems like there is little to no movement on supporting ESLint 9 upstream in eslint-config-airbnb-base
. Is it time we moved on to a different JavaScript standard?
Although there aren't any specific tests I don't see why this would cause any problems, let's ship it and hopefully improve performance.
Committed b323820 and pushed to 11.x. Thanks!
Yet more CI improvements, nice! Doesn't seem worth backporting this, so committed to 11.x only.
Committed 0b47ac3 and pushed to 11.x. Thanks!
Backported down to 10.4.x as a docs-only fix.
Committed and pushed 1935c084986 to 11.x and 8c7ffa560b6 to 11.1.x and 297b980640b to 10.5.x and c5aac1766a7 to 10.4.x. Thanks!
Actually this just looks like dead code? The property is only read, never written.
Actually no, something has changed in PHPStan, I get the same locally
Line lib/Drupal/Core/Test/TestSetupTrait.php (in context of
class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39)
------ -------------------------------------------------------------------------------------------------
137 Method
class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39::prepareDatabasePrefix()
has no return type specified.
🪪 missingType.return
146 Method
class@anonymous/tests/Drupal/Tests/Core/Test/TestSetupTraitTest.php:39::changeDatabasePrefix()
has no return type specified.
🪪 missingType.return
The bot is wrong.
Review bot can only cope with 11.x branches I believe.
I think all that needs to happen here is that the RMs have to agree that beta stability requirements have been met, and then we update the documentation to match.
TokuDB for MySQL looks to be abandoned since about 2015 and https://www.tokutek.com/ doesn't even resolve any more so there seems no point in keeping this open.
longwave → created an issue.
longwave → created an issue.
Discussed this with @penyaskito and @effulgentsia. We came up with two alternative options; we can still bikeshed over the exact names but JSON Schema has no opinion on how to handle extensions, so we need to make our own decision here.
- As proposed by @pdureau in #4 we use
meta:enum
for human readable enum values, and some othermeta:
prefix key for translation contexts. - Or, as per this JSON Schema blog post where we
added x-formatting-context to Experience Builder
📌
`StringSemanticsConstraint::MARKUP`: agree how SDC prop JSON schema can convey it should be markup, and
Needs review
we use something like
x-enum-labels
for human readable enum values andx-translation-contexts
for translation contexts.
@pdureau (or anyone else!) do you have any strong preferences or guidance on this?
@deviantintegral I haven't tested this at all but given that we support blocks as components, perhaps you just need https://www.drupal.org/project/entity_block → in order to embed any entity as a component?
The top level meta for this is 🌱 [Meta] Implement strict typing in existing code Active , adding this as a child and postponing. It is likely that we will do wider changes than interface-by-interface, but until we decide this issue can stay postponed.
+1 for skipping spellcheck on that file as it's machine generated anyway. The issue was resolved, back to RTBC.
Fixed a typo in the comments but otherwise this looks great, thanks for working through the pain of the update hooks and adding the test coverage for that - glad to see the back of this finally.
Looks good to me, tagging for change record updates as it does make sense to merge this with the other one - let's do that on commit.
The last 10.5.x had the usual array of random fails but at least one complete pipeline was fully green.
Looks good to me.. let's try to ship this with 11.2.
Fixed some invalid YAML which fails on newer Symfony. The other issues were problems in HEAD, fixed in 🐛 HEAD broken on 10.5.x Active so hopefully this run is green.
Reverted the above three from 10.5.x (and the chromedriver change from 10.4.x) which should hopefully make things pass again.
Reverted this from 10.5.x as it caused a failure in BookMultilingualTest.
Unsure if this is a problem with just the test or a problem with the patch, this wasn't spotted in 11.x because book.module has moved to contrib.
Reverted from 10.5.x and 10.4.x as this caused a failure in a FunctionalJavascript test.
In fact I just removed the bad test and tagged it with this issue number for better visibility.
The revert here was incorrect, we should not be re-adding core/tests/Drupal/Nightwatch/Tests/dialogDeprecations.js as it never existed in 10.5.x in the first place. ✨ Reduce jQuery usage Active was only committed to 11.x
Opened 🐛 HEAD broken on 10.5.x Active before I found this, let's just mark this as fixed and sort out 10.5.x over there.
BookMultilingualTest started with 🐛 LanguageNegotiationUrl unnecessarily adds domain to outbound URL's Active
BigPipeRegressionTest crash appears to be caused by 📌 CI: Using stale drupalci chromedriver image (`chromedriver` is stale, `webdriver-chromedriver` gets updates) Active , maybe we just revert to the old CI image there for now.
Nightwatch problem appears related to 🐛 [random test failure] jQuery Events Deprecation Tests (Tests/dialogDeprecations) Active although that was reverted?
longwave → created an issue.
longwave → created an issue.
This does feel like another unfortunate side effect of Drupal making a distinction between a module being available on disk and a module that is actually installed.
By moving it to its own library anyone who was using it has to make changes by attaching that library, right?
So if they have to make changes anyway maybe we just remove it instead, because this feels kinda useless on its own.
Backported down to 10.4.x just in case someone actually wants to run this script on an older version, although it shows how often we use this if it's been broken for 18 months.
Committed and pushed 61dfd43e036 to 11.x and 7b58a06d834 to 10.5.x and 6c1360bc84b to 11.1.x and c6ae57bfb72 to 10.4.x. Thanks!
@andypost wins Trivial Patch of the Month with a 1-bit diff!
Committed 906a9cb and pushed to 11.x. Thanks!
Let's do it, nice easy one here.
Committed 2ffe57e and pushed to 11.x. Thanks!
Is this really useful anywhere else? Can we just deprecate it and add the relevant parts directly to Claro's CSS for .tabs__trigger
?