Deprecate jquery.once and use the new once lib

Tests failures needs to go to 0 :)

Let's try this.

The patch is not clean, it includes the parent patch and the codemod I thrown in there to do the update. We should put the codemods in a better place and make it easy for contrib to use (like the upgrader module).

There are a few uses that can't be automatically updated.

There are optimisations we can do for certain usual forms like $(context).find('selector').once('toto').each(callback); let's just see how much it's breaking things.

Mainly deprecation, a couple of actual regressions

Yeah I was planning on putting something over on GitHub to start with. Kinda short on time these days :p

Ideally we'd have other people do npx drupal-upgrade and that makes the magic.

no more deprecations, let's see what we broke.

that went well :) small workaround in the interdiff until I update the lib: https://github.com/theodoreb/once/pull/3

update after the change to once.filter interdiff not so useful but it's there in case.

subltly broken things fix on the node preview.

still an issue with the media widget. There is a strange gray background around media fields, no idea where it comes from. Haven't looked closely at the media JS.

Agreed, was going to replace them manually later on. Started with the bigpipe one in the last interdiff. Wanted to have green tests before I change things manually too much.

thanks for the find, let's try this.

Looking good!

Did a first manual pass at fixing obviously inefficient replacements.

// From
$(once()).each((index, value) => {});
// To
once.forEach((value) => {});

# -----

// From
$(once()).each(function () { const $this = $(this); }));
// To
once().forEach((value) => { const $this = $(value); });

# -----

// From
const $els = $(once());
if (!$els.length) {
  return
}
const $other = $els.find();
// To
const els = once();
if (!els.length) {
  return
}
const $els = $(els);
const $other = $els.find();

I didn't touch the $(once()).on() because we don't have anything better to replace it with. Didn't mess too much with the selectors either because sometimes it's not sure that the jQuery object is a single element

// If we want to search for all 'selector' in those 3 elements: 
const els = [element1, element2, element3];
// it's trivial in jQuery
$(els).find('selector');
// In js that means doing something like
els.flatMap(el => Array.from(el.querySelectorAll('selector')));

I'd rather deal with that later on.

forgot to replace a "this"

last this missing.

Checked the loading perfs of before/after the patch. Thanks to the benchmark we already know once is faster than the jquery version, it holds up in real life even with quickedit doing a lot of once calls. I don't have formal numbers, its just slightly faster.

Updating the lib after some hardening on parameter validation.

Patch without the once lib. Should be green once parent patch is in (remove all the vendor/assets files from this patch)
Had to reroll for #3083051: Refactor tabledrag when core issues are resolved

other than that, no changes.

I still have the jscodeshift deps in this patch, we probably won't keep it on commit though.

Moving to a merge request for easier review/feedback.

Tagging because we talked about having integration tests for the once library. Leaving as need review because there is still a lot of code to go through independently of the tests

HOW TO REVIEW

There are 2 branches:

• The branch 3183149-manual-adjustments is the one to commit to. This holds all the change on top of the automated replacements.
  Merge request from fork:3183149-manual-adjustments → fork:3183149-auto-replace (for the diff between auto fix and manual changes)
  Merge request from fork:3183149-manual-adjustments → core:9.2.x (for core commit)
• The branch 3183149-auto-replace holds all the automated replacements (no human intervention on the code of this branch, see below for the list of replacement that are made automatically). It is used to show the diff between automated fixes and manual changes, so please do not commit to it.

The human code is on the branch 3183149-manual-adjustments There is a merge request created between 2 branches in the fork to facilitate discussion on what additional changes are needed on top of automated replacements: https://git.drupalcode.org/issue/drupal-3183149/-/merge_requests/1/diffs

taken care of #42 as part of this issue

Any takers for a review? let me know if you need time to talk it through directly or need more details about something.

Opened #3207782: Add BC layer between @drupal/once and jQuery.once to address BC concerns I wouldn't postpone this one, we already tried a few things for BC in the different once issues, there are several solutions that work, just a matter of agreeing on one.

going at it step by steps. Addressing #3208051: Use @drupal/once for the block module first to add some integration testing for the once library.

I feel it's ready too, as you may have guessed it's not a technical issue that is holding this patch up. It's just too big for someone that hasn't followed it from the start, so after talking with other core committer we decided to split a few pieces of "reviewable" size to let others get a sens of what happened here.

I'll spend time with justafish to go through the changes, and I'll answer questions until the rest of the core committers are comfortable with committing this patch. Large JS changes are still somewhat scary because we don't have many tests and most people are not familiar with the whole codebase, so it's an opportunity to get more people onboard :)

I know it's not going as fast as we'd like but looking back at the last 6 months, there's been some pretty significant changes already (on a personal note, I'll soon be able to contribute to core full-time so that could help). We're getting there, slowly but surely, going slowly here will make it easier for the other big patches that are sleeping in the queue :D

It's not strictly the same, once uses are not always about event listeners.

In your example they don't do the same thing, with once you bind an event listener once but it triggers every time you click on the element, with the code using the once option from the event listener, the event can be bound multiple times and will only trigger once.

So I don't see us retiring the once library as long as we rely on behaviors to initialize JS code.

have a look at #2367655: Control the list of behaviors run by Drupal.attachBehaviors and #1446166: Use JS events instead of Drupal.behaviors to see previous discussions. This is starting to become off topic though, so let's stick with the once thing here and discuss this in a separate issue, thanks.

updated to 9.3.x

the block issue was a spin-off to make onboarding easier on this patch. The plan is not to create one issue per module otherwise it'll never end. This patch has been green for 7 months already so just trying to make it happen ASAP :)

Tests are in #3207782: Add BC layer between @drupal/once and jQuery.once

Let me know if you want me to walk through the patch. Currently I need someone who'll be able to RTBC this so happy to spend time with folks to talk it through.

Dare I ask how much of this can/has been tested?

Well what you see in the IS, I wrote the codemod to automate the change, and each transformation has been written carefully to not replace things that shouldn't be replaced.

As far as manual changes go, I did test every change to check that the result was similar to previously (that's how I caught the problems with this in the view and ajax files).

There is also the fact that a bad patch does break some tests so there is at least some form of coverage. A few months ago I ran the Drupal tests for the JS coverage, the result is here: https://lonely-fifth.surge.sh/index.html it's surprisingly good, more than I expected. We added some more tests lately so coverage is even better.

Addressed lauriii comments.

credit from #3208051: Use @drupal/once for the block module