Deprecate domready and remove usages in core

1. +++ b/core/lib/Drupal/Core/Render/theme.api.php
   @@ -1039,8 +1039,6 @@ function hook_css_alter(&$css, \Drupal\Core\Asset\AttachedAssetsInterface $asset
    function hook_page_attachments(array &$attachments) {
   -  // Unconditionally attach an asset to the page.
   -  $attachments['#attached']['library'][] = 'core/domready';
   

   Let's keep this in but let's switch to another library. This is just example code for documentation.

2. +++ b/core/tests/Drupal/KernelTests/Core/Asset/AttachedAssetsTest.php
   @@ -235,7 +235,6 @@ public function testHeaderHTML() {
   -    $this->assertNotIdentical(strpos($rendered_js, '<script src="' . file_url_transform_relative(file_create_url('core/assets/vendor/domready/ready.min.js'))), FALSE, 'The JS asset of the indirect dependency (core/domready) of common_test/js-header appears in the header.');
   

   Maybe we should make the common_test/js-header depend on another library that has nested dependencies?

1. +++ b/core/lib/Drupal/Core/Render/theme.api.php
   @@ -1040,7 +1040,7 @@ function hook_css_alter(&$css, \Drupal\Core\Asset\AttachedAssetsInterface $asset
   -  $attachments['#attached']['library'][] = 'core/domready';
   +  $attachments['#attached']['library'][] = 'core/drupalSettings';
   
   +++ b/core/tests/Drupal/KernelTests/Core/Asset/AttachedAssetsTest.php
   @@ -235,7 +235,7 @@ public function testHeaderHTML() {
   -    $this->assertNotIdentical(strpos($rendered_js, '<script src="' . file_url_transform_relative(file_create_url('core/assets/vendor/domready/ready.min.js'))), FALSE, 'The JS asset of the indirect dependency (core/domready) of common_test/js-header appears in the header.');
   +    $this->assertNotIdentical(strpos($rendered_js, '<script src="' . file_url_transform_relative(file_create_url('core/misc/drupalSettingsLoader.js'))), FALSE, 'The JS asset of the indirect dependency (core/drupalSettings) of common_test/js-header appears in the header.');
   

   Nice 👍

2. +++ b/core/misc/drupal.init.es6.js
   @@ -6,12 +6,29 @@ if (window.jQuery) {
   +    document.addEventListener('DOMContentLoaded', listener);
   

   Double checked that the event is supported by Drupals supported browser. Looks all good: https://developer.mozilla.org/en-US/docs/Web/API/Window/DOMContentLoaded...

3. +++ b/core/misc/drupal.init.es6.js
   @@ -6,12 +6,29 @@ if (window.jQuery) {
   +Drupal.domReady = callback => {
   

   We don't have any explicit test coverage for this but that's probably fine since if this doesn't work, most Drupal behaviors will be broken.

   Given the implications of this not working, it would be probably good to do a bit of manual testing on this across our supported browsers. Other than that, this looks good.

Couple of things.

As is the Drupal.domready is not overridable. We define it in the file then immediately use it, there is no chance for anything else to override the function. if we want to make it overridable the definition of Drupal.domready needs to be in a separate file like the current domready lib.

Usually we declare everything inside the closure so Drupal.domReady sould be inside the closure.

The domready lib can handle several callbacks, whereas this version can't.

Given that I'd be up to not make the domready thing overridable and just go with how #2 declares a onReady function inside the closure. It's simply a way to trigger behaviors which are what should be used to do things anyway.

Thanks, ok for me now.

The domready lib can handle several callbacks, whereas this version can't.

Good catch! 👍Thank you @nod_!

Awesome! Thanks for testing the patch manually on a broad range of browsers!

After this has been committed, we could update the change record to link to a Gitlab page of this file to show this function so that people who need this, could copy it from there.

The code itself looks good. The deprecation message links to the correct change record and the replacing function looks fine. It has been also reviewed by @nod_ who is a JavaScript subsystem maintainer. Moving to RTBC and tagging with needs release manager review. 👏

Committed and pushed to 9.0.x, 8.9.x, and 8.8.x, and published the change record. Thanks!

I didn't quite follow the updates that were to be made to https://www.drupal.org/node/3086669 but I think they probably involve putting in the name of the new function and linking it to the source code?

@xjm I updated the change record with what I thought at the time of writing that comment. We can't really reference to a function name because it's just a local variable.