Problem/Motivation

Change record: Remove jQuery dependency from the once feature

Steps to reproduce

  1. install Drupal 10
  2. composer require drupal/typed_data:^1.x-dev@dev
  3. composer require drupal/rules:^3.x-dev@dev
  4. drush pm:enable rules
  5. create a new rule
  6. the data selector never shows autocompleted results

Notes

  • the js/autocomplete.js script uses jQuery once which is no more supported in Drupal 10 and should be replaced with once
  • JavaScript console says "Uncaught TypeError: $(...).find(...).once is not a function" at line 110.

Issue fork rules-3338673

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Zigazou created an issue. See original summary.

tr’s picture

Title: Data selector (autocomplete.js) has not been updated to Drupal 10 » Remove jQuery dependency from the once feature
Issue summary: View changes
Issue tags: -Drupal 10 compatibility, -autocomplete

@Zigazou: Can you supply a patch?

_pratik_’s picture

Status: Active » Needs review
StatusFileSize
new1.47 KB
zigazou’s picture

Hi!

After applying the patch, the '$' variable is no more available and triggers the following message:

Uncaught TypeError: $ is undefined
    attach https://sandbox.local/modules/contrib/rules/js/autocomplete.js?rpnskb:115
tr’s picture

Status: Needs review » Needs work

We use jquery.once in three different .js files:

  • debug.js
  • rules_ui.listing.js
  • autocomplete.js

but it appears the above patch only modifies one of these (autocomplete.js).

According to the comments in autocomplete.js, Rules autocomplete.js was forked from core misc/autocomplete.js. I would have to do research to find out why it was forked, and what Rules is doing different, but if at all possible the Rules version should be changed the same way as the core version.

For reference, jQuery once was removed from core autocomplete in #3183149: Deprecate jquery.once and use the new once lib.

For this patch, I would also like to evaluate the dependencies for all of these .js files. For example, with autocomplete.js, right now we use:

rules.autocomplete:
  js:
    js/autocomplete.js: { weight: -1 }
  dependencies:
    - core/drupal
    - core/drupalSettings
    - core/drupal.ajax
    - core/jquery
    - core/jquery.once
    - core/jquery.ui.autocomplete

but I suspect that most of these are not needed and should be removed. Same with the other .js.

zigazou’s picture

StatusFileSize
new6.16 KB

Hi!

Following comment #5 saying rules/autocomplete.js was forked, I did the same with Drupal 10 core/misc/autocomplete.js.

From what I saw, the custom autocomplete.js:

  • immediately pop-up autocomplete suggestions when the field gets focus
  • uses rules-* classes instead of form-* classes
  • starts autocompletion with a minimum length of 0

I did not look at debug.js and rules_ui_listing.js.

Can someone please review the patch?

tr’s picture

Title: Remove jQuery dependency from the once feature » [10.0] Remove jQuery dependency from the once feature

As you can see from the tests, your patch didn't fix any of the failures that are caused by using the (now removed) jquery-once library.

tr’s picture

rules_ui.listing.js was fixed in #3346889: [meta] Update rules_ui.listing.js

We still need to fix both debug.js and autocomplete.js

tr’s picture

Status: Needs work » Needs review
StatusFileSize
new7.17 KB

Here's a re-roll with fixes to debug.js.

tr’s picture

StatusFileSize
new3.9 KB
new8.85 KB

The remaining D10 error is not related to this patch.
Here's some more cleanup.

Note - autocomplete.js and debug.js both require manual testing, because we don't have any test cases that use these. I could really use help manually testing these.

hongqing’s picture

I tested patch #10, it works well with PHP 8.1, MySql 5.7, and Drupal 10.0.9. Thanks.

tr’s picture

StatusFileSize
new9.1 KB
new713 bytes

Fixed two coding standards errors and added documentation comments to explain why autocomplete.js was forked, based on what @Zigazou said in #6.

keshav.k made their first commit to this issue’s fork.

keshavv’s picture

created MR for the patch please review.

  • TR committed 360dff42 on 8.x-3.x
    Issue #3338673 by TR, Zigazou: [10.0] Remove jQuery dependency from the...
tr’s picture

Status: Needs review » Fixed

Committed.

tr’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.