Problem/Motivation
The jQuery UI is being deprecated for Drupal 9 and therefore, we need to convert it to use the new SortableJS library as written in this deprecation page: jQuery UI Sortable deprecated and core usages removed. A quick search shows that it is listed as dependency for entity_reference and entity_reference.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | counter.mov | 5.37 MB | oknate |
| #14 | 3110133-14.patch | 14.15 KB | oknate |
| #14 | 3110133-interdiff--13-14.txt | 1.4 KB | oknate |
| #13 | interdiff-3110133-11-13.txt | 2.27 KB | phenaproxima |
| #13 | 3110133-13.patch | 14.17 KB | phenaproxima |
Comments
Comment #2
sasanikolic commentedHere is the patch with the converted library to SortableJS.
Comment #4
sasanikolic commentedTests failed because a call to uniqueId(), which comes from jQuery UI which is removed and will be deprecated. Had to replace it with another unique ID generator, so I decided to use Crypto.getRandomValues(), as it seems like there's nothing like that in Drupal core's javascript that we could use. Let me know if there is a better solution to it.
Comment #6
berdirFWIW, uniqueId() isn't actually a _random_ id, it's just an incrementing counter: https://github.com/jquery/jquery-ui/blob/74f8a0ac952f6f45f773312292baef1...
This might be fine, but in case that makes it easier, we could also do something like that.
Comment #7
berdir> 1) Drupal\Tests\entity_browser\FunctionalJavascript\EntityReferenceWidgetTest::testEntityReferenceWidget
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "entity.query". Did you mean one of these: "entity.query.config", "entity.query.sql", "pgsql.entity.query.sql", "entity.query.null", "entity.query.keyvalue"?
Noticed this one in the test fails, that's from:
$nid = $this->container->get('entity.query')->get('node')->condition('title', 'Referencing node 1')->execute();
I guess there's no @trigger_error() for the entity.query service and without this, we afaik don't even get far enough in the test to trigger it, so maybe just fix it here? just use \Drupal::entityQuery('node')->condition...
Comment #8
berdirI did the entity query thing in #3111564: Update/Remove EntityBrowserUpdateHookTest for D9 compatibility and other test fixes now.
Comment #9
phenaproximaHere's some iterating on #4. Let's see if it helps with the D9 test failures.
Comment #11
phenaproximaThis should get EntityReferenceWidgetTest to pass; I modified it to use SortableTestTrait.
Comment #13
phenaproximaLucky #13. 😬😲🤭 This fixed the remaining failure for me locally; let's see how it fares.
Comment #14
oknateAs per Berdir's recommendation, replacing the crypto counter with a simple counter. I've added a video with the patch (with an additional debug statement to show the counting).
Other than that I think this is ready to go. Sorting seems to work fine.
Comment #15
phenaproximaIt looks like this module already fails tests against PostgreSQL, so this patch doesn't appear to be introducing problems there.
Given that, if it passes on MySQL for 8.8, 8.9, and 9.0, it's ready IMHO. I'm not really supposed to RTBC my own work, but who else will? So let's do this, on the presumption that those runs will come back green.
Comment #16
phenaproximaThis issue flat-out blocks Drupal 9 compatibility, since core no longer ships with jQuery UI. Therefore, I'm escalating its priority, and IMHO this should be critical (but I'll leave that decision to the maintainers).
Comment #18
oknateCommitted.